Added error checking code to Collection and Subcollection classes

Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuhic@capeannenterprises.com>

diff --git a/sdk/R/R/Arvados.R b/sdk/R/R/Arvados.R
index b21c6046356b1eb73503a4cd846ed4cd43b81312..6493b899afa73cbf5c8575b277388ef46a216fbe 100644 (file)
--- a/sdk/R/R/Arvados.R
+++ b/sdk/R/R/Arvados.R
@@ -25,12 +25,12 @@ Arvados <- R6::R6Class(
                 Sys.setenv(ARVADOS_API_TOKEN = authToken)
 
             hostName  <- Sys.getenv("ARVADOS_API_HOST");
-            token <- Sys.getenv("ARVADOS_API_TOKEN");
+            token     <- Sys.getenv("ARVADOS_API_TOKEN");
 
             if(hostName == "" | token == "")
-                stop(paste0("Please provide host name and authentification token",
-                            " or set ARVADOS_API_HOST and ARVADOS_API_TOKEN",
-                            " environment variables."))
+                stop(paste("Please provide host name and authentification token",
+                           "or set ARVADOS_API_HOST and ARVADOS_API_TOKEN",
+                           "environment variables."))
 
             private$REST  <- RESTService$new(token, hostName, NULL,
                                              HttpRequest$new(), HttpParser$new())
@@ -119,8 +119,7 @@ Arvados <- R6::R6Class(
             names(body) <- c("group")
             body$group <- newContent
 
-            updatedProject <- private$REST$updateResource("groups",
-                                                          uuid, body)
+            updatedProject <- private$REST$updateResource("groups", uuid, body)
             updatedProject
         },
 
@@ -131,8 +130,7 @@ Arvados <- R6::R6Class(
 
             filters[[length(filters) + 1]] <- list("group_class", "=", "project")
 
-            projects <- private$REST$listResources("groups", filters,
-                                                   limit, offset)
+            projects <- private$REST$listResources("groups", filters, limit, offset)
             projects
         },
 
@@ -169,7 +167,7 @@ Arvados <- R6::R6Class(
 #' @export print.Arvados
 print.Arvados = function(arvados)
 {
-    cat(paste0("Type:  ", "\"", "Arvados", "\""), sep = "\n")
+    cat(paste0("Type:  ", "\"", "Arvados",             "\""), sep = "\n")
     cat(paste0("Host:  ", "\"", arvados$getHostName(), "\""), sep = "\n")
-    cat(paste0("Token: ", "\"", arvados$getToken(), "\"") , sep = "\n")
+    cat(paste0("Token: ", "\"", arvados$getToken(),    "\""), sep = "\n")
 }

diff --git a/sdk/R/R/ArvadosFile.R b/sdk/R/R/ArvadosFile.R
index 7ffcb294183c55c6216f396cdce1995153cb0058..3437933a7388df84f8b135826a26160293151856 100644 (file)
--- a/sdk/R/R/ArvadosFile.R
+++ b/sdk/R/R/ArvadosFile.R
@@ -13,9 +13,10 @@ ArvadosFile <- R6::R6Class(
 
         initialize = function(name)
         {
-            private$name             <- name
-            private$http             <- HttpRequest$new()
-            private$httpParser       <- HttpParser$new()
+            if(name == "")
+                stop("Invalid name.")
+
+            private$name <- name
         },
 
         getName = function() private$name,
@@ -132,7 +133,6 @@ ArvadosFile <- R6::R6Class(
             if(is.null(private$collection))
                 stop("ArvadosFile doesn't belong to any collection")
 
-
             newLocation <- trimFromEnd(newLocation, "/")
             nameAndPath <- splitToPathAndName(newLocation)
 
@@ -168,8 +168,6 @@ ArvadosFile <- R6::R6Class(
         size       = NULL,
         parent     = NULL,
         collection = NULL,
-        http       = NULL,
-        httpParser = NULL,
         buffer     = NULL,
 
         attachToNewParent = function(newParent)
@@ -215,8 +213,8 @@ print.ArvadosFile = function(arvadosFile)
         relativePath <- paste0("/", relativePath)
     }
 
-    cat(paste0("Type:          ", "\"", "ArvadosFile", "\""), sep = "\n")
+    cat(paste0("Type:          ", "\"", "ArvadosFile",         "\""), sep = "\n")
     cat(paste0("Name:          ", "\"", arvadosFile$getName(), "\""), sep = "\n")
-    cat(paste0("Relative path: ", "\"", relativePath, "\"") , sep = "\n")
-    cat(paste0("Collection:    ", "\"", collection, "\""), sep = "\n")
+    cat(paste0("Relative path: ", "\"", relativePath,          "\""), sep = "\n")
+    cat(paste0("Collection:    ", "\"", collection,            "\""), sep = "\n")
 }

diff --git a/sdk/R/R/Collection.R b/sdk/R/R/Collection.R
index 210762043526396446f9cf92e389e54ddf43f392..47d88ac0bc9fd3980f75d6a2b662c36d19495cbe 100644 (file)
--- a/sdk/R/R/Collection.R
+++ b/sdk/R/R/Collection.R
@@ -49,6 +49,10 @@ Collection <- R6::R6Class(
             if("ArvadosFile"   %in% class(content) ||
                "Subcollection" %in% class(content))
             {
+
+                if(content$getName() == "")
+                    stop("Content has invalid name.")
+
                 subcollection$add(content)
                 content
             }
@@ -118,8 +122,14 @@ Collection <- R6::R6Class(
                         stop(paste("File", filePath, "doesn't exist."))
 
                     parent <- file$getParent()
+
+                    if(is.null(parent))
+                        stop("You can't delete root folder.")
+
                     parent$remove(file$getName())
                 })
+
+                "Content removed"
             }
             else 
             {
@@ -170,5 +180,5 @@ Collection <- R6::R6Class(
 print.Collection = function(collection)
 {
     cat(paste0("Type: ", "\"", "Arvados Collection", "\""), sep = "\n")
-    cat(paste0("uuid: ", "\"", collection$uuid, "\""), sep = "\n")
+    cat(paste0("uuid: ", "\"", collection$uuid,      "\""), sep = "\n")
 }

diff --git a/sdk/R/R/RESTService.R b/sdk/R/R/RESTService.R
index 65c5302ba45208c6d7f12d73caa29bf7e940f79c..62356b76384094a8ba0d222dedd9ec77d2fb7f03 100644 (file)
--- a/sdk/R/R/RESTService.R
+++ b/sdk/R/R/RESTService.R
@@ -174,7 +174,7 @@ RESTService <- R6::R6Class(
             if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
 
-            paste("File deleted:", relativePath)
+            serverResponse
         },
 
         move = function(from, to, uuid)

diff --git a/sdk/R/R/Subcollection.R b/sdk/R/R/Subcollection.R
index 7eb4381edaa151b2ef1053a78c039dfd7af4150d..d580695e61e2881025fd5c2f927395b49e045e34 100644 (file)
--- a/sdk/R/R/Subcollection.R
+++ b/sdk/R/R/Subcollection.R
@@ -38,7 +38,11 @@ Subcollection <- R6::R6Class(
             if("ArvadosFile"   %in% class(content) ||
                "Subcollection" %in% class(content))
             {
+                if(content$getName() == "")
+                    stop("Content has invalid name.")
+
                 childWithSameName <- self$get(content$getName())
+
                 if(!is.null(childWithSameName))
                     stop(paste("Subcollection already contains ArvadosFile",
                                "or Subcollection with same name."))
@@ -83,6 +87,7 @@ Subcollection <- R6::R6Class(
                 {
                     REST <- private$collection$getRESTService()
                     REST$delete(child$getRelativePath(), private$collection$uuid)
+
                     child$setCollection(NULL)
                 }
 
@@ -102,7 +107,6 @@ Subcollection <- R6::R6Class(
         getFileListing = function(fullPath = TRUE)
         {
             content <- private$getContentAsCharVector(fullPath)
-
             content[order(tolower(content))]
         },
 
@@ -257,7 +261,6 @@ Subcollection <- R6::R6Class(
             }
 
             content
-
         }
     ),
     
@@ -280,6 +283,6 @@ print.Subcollection = function(subCollection)
 
     cat(paste0("Type:          ", "\"", "Arvados Subcollection", "\""), sep = "\n")
     cat(paste0("Name:          ", "\"", subCollection$getName(), "\""), sep = "\n")
-    cat(paste0("Relative path: ", "\"", relativePath, "\"") , sep = "\n")
-    cat(paste0("Collection:    ", "\"", collection, "\""), sep = "\n")
+    cat(paste0("Relative path: ", "\"", relativePath,            "\""), sep = "\n")
+    cat(paste0("Collection:    ", "\"", collection,              "\""), sep = "\n")
 }

diff --git a/sdk/R/tests/testthat/test-ArvadosFile.R b/sdk/R/tests/testthat/test-ArvadosFile.R
index 43c841bf8ce7906e2261a29e8e497173a57fcf5c..90cc1499d674df18f075876c4e2ba0bf028f06e7 100644 (file)
--- a/sdk/R/tests/testthat/test-ArvadosFile.R
+++ b/sdk/R/tests/testthat/test-ArvadosFile.R
@@ -2,6 +2,11 @@ source("fakes/FakeRESTService.R")
 
 context("ArvadosFile")
 
+test_that("constructor raises error if  file name is empty string", {
+
+    expect_that(ArvadosFile$new(""), throws_error("Invalid name."))
+}) 
+
 test_that("getFileListing always returns file name", {
 
     dog <- ArvadosFile$new("dog")
@@ -207,6 +212,7 @@ test_that(paste("move raises exception if newLocationInCollection",
                            "animal/dog",
                            "animal/fish/shark",
                            "ball")
+
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -227,6 +233,7 @@ test_that("move raises exception if new location contains content with the same
                            "animal/dog",
                            "animal/fish/shark",
                            "dog")
+
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -247,6 +254,7 @@ test_that("move moves arvados file inside collection tree", {
                            "animal/dog",
                            "animal/fish/shark",
                            "ball")
+
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")

diff --git a/sdk/R/tests/testthat/test-Collection.R b/sdk/R/tests/testthat/test-Collection.R
index 63a402dba4e358045c4e0f75de0272d19679ece4..ec00ca3c66dbcc66d875abee3d810a9ba06b9cd9 100644 (file)
--- a/sdk/R/tests/testthat/test-Collection.R
+++ b/sdk/R/tests/testthat/test-Collection.R
@@ -5,10 +5,7 @@ context("Collection")
 test_that(paste("constructor creates file tree from text content",
                 "retreived form REST service"), {
 
-
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -24,9 +21,7 @@ test_that(paste("constructor creates file tree from text content",
 test_that(paste("add raises exception if passed argumet is not",
                 "ArvadosFile or Subcollection"), {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -42,9 +37,7 @@ test_that(paste("add raises exception if passed argumet is not",
 
 test_that("add raises exception if relative path is not valid", {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -58,12 +51,25 @@ test_that("add raises exception if relative path is not valid", {
                               fixed = TRUE))
 })
 
+test_that("add raises exception if content name is empty string", {
+
+    collectionContent <- c("animal", "animal/fish")
+    fakeREST <- FakeRESTService$new(collectionContent)
+
+    api <- Arvados$new("myToken", "myHostName")
+    api$setRESTService(fakeREST)
+    collection <- Collection$new(api, "myUUID")
+
+    rootFolder <- Subcollection$new("")
+
+    expect_that(collection$add(rootFolder),
+                throws_error("Content has invalid name.", fixed = TRUE))
+})
+
 test_that(paste("add adds ArvadosFile or Subcollection",
                 "to local tree structure and remote REST service"), {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -82,9 +88,7 @@ test_that(paste("add adds ArvadosFile or Subcollection",
 
 test_that("create raises exception if passed argumet is not character vector", {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -101,6 +105,7 @@ test_that("create raises exception if relative path is not valid", {
     collectionContent <- c("animal",
                            "animal/fish",
                            "ball")
+
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -117,9 +122,7 @@ test_that("create raises exception if relative path is not valid", {
 test_that(paste("create adds files specified by fileNames",
                 "to local tree structure and remote REST service"), {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -141,9 +144,7 @@ test_that(paste("create adds files specified by fileNames",
 
 test_that("remove raises exception if passed argumet is not character vector", {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -155,14 +156,23 @@ test_that("remove raises exception if passed argumet is not character vector", {
                              fixed = TRUE))
 })
 
+test_that("remove raises exception if user tries to remove root folder", {
+
+    collectionContent <- c("animal", "animal/fish")
+    fakeREST <- FakeRESTService$new(collectionContent)
+
+    api <- Arvados$new("myToken", "myHostName")
+    api$setRESTService(fakeREST)
+    collection <- Collection$new(api, "myUUID")
+
+    expect_that(collection$remove(""),
+                throws_error("You can't delete root folder.", fixed = TRUE))
+})
+
 test_that(paste("remove removes files specified by paths",
                 "from local tree structure and from remote REST service"), {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "animal/dog",
-                           "animal/cat",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "animal/dog", "animal/cat", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -184,9 +194,7 @@ test_that(paste("remove removes files specified by paths",
 test_that(paste("move moves content to a new location inside file tree",
                 "and on REST service"), {
 
-    collectionContent <- c("animal",
-                           "animal/dog",
-                           "ball")
+    collectionContent <- c("animal", "animal/dog", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -205,9 +213,7 @@ test_that(paste("move moves content to a new location inside file tree",
 
 test_that("move raises exception if new location is not valid", {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -221,9 +227,7 @@ test_that("move raises exception if new location is not valid", {
 
 test_that("getFileListing returns sorted collection content received from REST service", {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")
@@ -241,9 +245,7 @@ test_that("getFileListing returns sorted collection content received from REST s
 
 test_that("get returns arvados file or subcollection from internal tree structure", {
 
-    collectionContent <- c("animal",
-                           "animal/fish",
-                           "ball")
+    collectionContent <- c("animal", "animal/fish", "ball")
     fakeREST <- FakeRESTService$new(collectionContent)
 
     api <- Arvados$new("myToken", "myHostName")

diff --git a/sdk/R/tests/testthat/test-Subcollection.R b/sdk/R/tests/testthat/test-Subcollection.R
index b155ed4ccbcafd2f2badd4170dd9f7f319b41e0b..1b141e10514c91e3db63ad0368989141137726ff 100644 (file)
--- a/sdk/R/tests/testthat/test-Subcollection.R
+++ b/sdk/R/tests/testthat/test-Subcollection.R
@@ -73,6 +73,15 @@ test_that("add adds content to inside collection tree", {
     expect_that(animalContainsDog, is_true())
 }) 
 
+test_that("add raises exception if content name is empty string", {
+
+    animal     <- Subcollection$new("animal")
+    rootFolder <- Subcollection$new("")
+
+    expect_that(animal$add(rootFolder),
+                throws_error("Content has invalid name.", fixed = TRUE))
+})
+
 test_that(paste("add raises exception if ArvadosFile/Subcollection", 
                 "with same name already exists in the subcollection"), {