Fixed some bugs and improved error handling.

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

diff --git a/sdk/R/R/ArvadosFile.R b/sdk/R/R/ArvadosFile.R
index 85d11c7d66b7f28fe3d471a9648e93ae47715ed1..84e76af4517200b756b2b393b78913aa423459d8 100644 (file)
--- a/sdk/R/R/ArvadosFile.R
+++ b/sdk/R/R/ArvadosFile.R
@@ -11,9 +11,9 @@ ArvadosFile <- R6::R6Class(
 
         initialize = function(name)
         {
-            private$name       <- name
-            private$http       <- HttpRequest$new()
-            private$httpParser <- HttpParser$new()
+            private$name             <- name
+            private$http             <- HttpRequest$new()
+            private$httpParser       <- HttpParser$new()
         },
 
         getName = function() private$name,
@@ -39,13 +39,14 @@ ArvadosFile <- R6::R6Class(
         removeFromCollection = function()
         {
             if(is.null(private$collection))
-                stop("Subcollection doesn't belong to any collection.")
+                stop("ArvadosFile doesn't belong to any collection.")
             
             private$collection$.__enclos_env__$private$deleteFromREST(self$getRelativePath())
 
-            #todo rename this add to a collection
             private$addToCollection(NULL)
             private$detachFromParent()
+
+            "Content removed successfully."
         },
 
         getRelativePath = function()
@@ -53,7 +54,6 @@ ArvadosFile <- R6::R6Class(
             relativePath <- c(private$name)
             parent <- private$parent
 
-            #Recurse back to root
             while(!is.null(parent))
             {
                 relativePath <- c(parent$getName(), relativePath)
@@ -66,11 +66,16 @@ ArvadosFile <- R6::R6Class(
 
         getParent = function() private$parent,
 
-        read = function(offset = 0, length = 0)
+        read = function(contentType = "raw", offset = 0, length = 0)
         {
-            #todo range is wrong fix it
+            if(is.null(private$collection))
+                stop("ArvadosFile doesn't belong to any collection.")
+
             if(offset < 0 || length < 0)
-            stop("Offset and length must be positive values.")
+                stop("Offset and length must be positive values.")
+
+            if(!(contentType %in% private$http$validContentTypes))
+                stop("Invalid contentType. Please use text or raw.")
 
             range = paste0("bytes=", offset, "-")
 
@@ -78,20 +83,31 @@ ArvadosFile <- R6::R6Class(
                 range = paste0(range, offset + length - 1)
             
             fileURL = paste0(private$collection$api$getWebDavHostName(), "c=", private$collection$uuid, "/", self$getRelativePath());
-            headers <- list(Authorization = paste("OAuth2", private$collection$api$getToken()), 
-                            Range = range)
+
+            if(offset == 0 && length == 0)
+            {
+                headers <- list(Authorization = paste("OAuth2", private$collection$api$getToken())) 
+            }
+            else
+            {
+                headers <- list(Authorization = paste("OAuth2", private$collection$api$getToken()), 
+                                Range = range)
+            }
 
             serverResponse <- private$http$GET(fileURL, headers)
 
-            if(serverResponse$status_code != 206)
+            if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
 
-            parsedServerResponse <- httr::content(serverResponse, "raw")
+            parsedServerResponse <- httr::content(serverResponse, contentType)
             parsedServerResponse
         },
         
         write = function(content, contentType = "text/html")
         {
+            if(is.null(private$collection))
+                stop("ArvadosFile doesn't belong to any collection.")
+
             fileURL = paste0(private$collection$api$getWebDavHostName(), "c=", private$collection$uuid, "/", self$getRelativePath());
             headers <- list(Authorization = paste("OAuth2", private$collection$api$getToken()), 
                             "Content-Type" = contentType)
@@ -99,7 +115,7 @@ ArvadosFile <- R6::R6Class(
 
             serverResponse <- private$http$PUT(fileURL, headers, body)
 
-            if(serverResponse$status_code != 201)
+            if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
 
             parsedServerResponse <- httr::content(serverResponse, "text")
@@ -108,6 +124,9 @@ ArvadosFile <- R6::R6Class(
 
         move = function(newLocation)
         {
+            if(is.null(private$collection))
+                stop("ArvadosFile doesn't belong to any collection.")
+
             if(endsWith(newLocation, paste0(private$name, "/")))
             {
                 newLocation <- substr(newLocation, 0, nchar(newLocation) - nchar(paste0(private$name, "/")))
@@ -125,25 +144,25 @@ ArvadosFile <- R6::R6Class(
 
             if(is.null(newParent))
             {
-                stop("Unable to get destination subcollectin")
+                stop("Unable to get destination subcollection.")
             }
 
-            status <- private$collection$.__enclos_env__$private$moveOnRest(self$getRelativePath(), paste0(newParent$getRelativePath(), "/", self$getName()))
+            status <- private$collection$.__enclos_env__$private$moveOnREST(self$getRelativePath(), paste0(newParent$getRelativePath(), "/", self$getName()))
 
             private$attachToParent(newParent)
 
-            paste("Status code :", status$status_code)
+            "Content moved successfully."
         }
     ),
 
     private = list(
 
-        name         = NULL,
-        size         = NULL,
-        parent       = NULL,
-        collection   = NULL,
-        http         = NULL,
-        httpParser   = NULL,
+        name       = NULL,
+        size       = NULL,
+        parent     = NULL,
+        collection = NULL,
+        http       = NULL,
+        httpParser = NULL,
 
         getChild = function(name)
         {
@@ -157,7 +176,7 @@ ArvadosFile <- R6::R6Class(
 
         addToCollection = function(collection)
         {
-            private$collection = collection
+            private$collection <- collection
         },
 
         detachFromParent = function()

diff --git a/sdk/R/R/Collection.R b/sdk/R/R/Collection.R
index ea6f692ce556c558c62fe71005ef384d2d2657e6..ca26082e076af04615ef304690634c78bf3e8f60 100644 (file)
--- a/sdk/R/R/Collection.R
+++ b/sdk/R/R/Collection.R
@@ -160,7 +160,7 @@ Collection <- R6::R6Class(
 
             serverResponse <- private$http$PUT(fileURL, headers, body)
 
-            if(serverResponse$status_code != 201)
+            if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
 
             print(paste("File created:", relativePath))
@@ -173,13 +173,13 @@ Collection <- R6::R6Class(
 
             serverResponse <- private$http$DELETE(fileURL, headers)
 
-            if(serverResponse$status_code != 204)
+            if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
 
-            print(paste("File deleted", relativePath))
+            print(paste("File deleted:", relativePath))
         },
 
-        moveOnRest = function(from, to)
+        moveOnREST = function(from, to)
         {
             collectionURL <- URLencode(paste0(self$api$getWebDavHostName(), "c=", self$uuid, "/"))
             fromURL <- paste0(collectionURL, from)

diff --git a/sdk/R/R/HttpRequest.R b/sdk/R/R/HttpRequest.R
index 7a399a4b30ff1ea4a7f75b1b2331aa102e847e20..83669072263249a91b6274d6bc0fda2463fb64c0 100644 (file)
--- a/sdk/R/R/HttpRequest.R
+++ b/sdk/R/R/HttpRequest.R
@@ -4,8 +4,11 @@ HttpRequest <- R6::R6Class(
 
     public = list(
 
+        validContentTypes = NULL,
+
         initialize = function() 
         {
+            self$validContentTypes <- c("text", "raw")
         },
 
         GET = function(url, headers = NULL, queryFilters = NULL, limit = NULL, offset = NULL)

diff --git a/sdk/R/R/Subcollection.R b/sdk/R/R/Subcollection.R
index a1fba1a4adf912f896fedbf26f152ca6b9ee269c..76bec2e64fc2117dedb461970bdad316845ac989 100644 (file)
--- a/sdk/R/R/Subcollection.R
+++ b/sdk/R/R/Subcollection.R
@@ -24,6 +24,10 @@ Subcollection <- R6::R6Class(
                 if(!is.null(content$.__enclos_env__$private$collection))
                     stop("ArvadosFile/Subcollection already belongs to a collection.")
 
+                childWithSameName <- private$getChild(content$getName())
+                if(!is.null(childWithSameName))
+                stop("Subcollection already contains ArvadosFile or Subcollection with same name.")
+
                 if(!is.null(private$collection))
                 {       
                     contentPath <- paste0(self$getRelativePath(), "/", content$getFileList())
@@ -33,10 +37,12 @@ Subcollection <- R6::R6Class(
 
                 private$children <- c(private$children, content)
                 content$.__enclos_env__$private$parent = self
+
+                "Content added successfully."
             }
             else
             {
-                stop("Expected AravodsFile or Subcollection object, got ...")
+                stop(paste("Expected AravodsFile or Subcollection object, got", class(content), "."))
             }
         },
 
@@ -45,7 +51,7 @@ Subcollection <- R6::R6Class(
             if(is.null(private$collection))
                 stop("Subcollection doesn't belong to any collection.")
 
-            if(self$name == "")
+            if(private$name == "")
                 stop("Unable to delete root folder.")
 
             collectionList <- paste0(self$getRelativePath(), "/", self$getFileList(fullpath = FALSE))
@@ -56,6 +62,8 @@ Subcollection <- R6::R6Class(
 
             private$addToCollection(NULL)
             private$dettachFromParent()
+
+            "Content removed successfully."
         },
 
         getFileList = function(fullpath = TRUE)
@@ -114,6 +122,9 @@ Subcollection <- R6::R6Class(
 
         move = function(newLocation)
         {
+            if(is.null(private$collection))
+                stop("Subcollection doesn't belong to any collection.")
+
             if(endsWith(newLocation, paste0(private$name, "/")))
             {
                 newLocation <- substr(newLocation, 0, nchar(newLocation) - nchar(paste0(private$name, "/")))
@@ -131,14 +142,14 @@ Subcollection <- R6::R6Class(
 
             if(is.null(newParent))
             {
-                stop("Unable to get destination subcollectin")
+                stop("Unable to get destination subcollection.")
             }
 
-            status <- private$collection$.__enclos_env__$private$moveOnRest(self$getRelativePath(), paste0(newParent$getRelativePath(), "/", self$getName()))
+            status <- private$collection$.__enclos_env__$private$moveOnREST(self$getRelativePath(), paste0(newParent$getRelativePath(), "/", self$getName()))
 
             private$attachToParent(newParent)
 
-            paste("Status code :", status$status_code)
+            "Content moved successfully."
         },
 
         getParent = function() private$parent
@@ -209,8 +220,11 @@ Subcollection <- R6::R6Class(
 
         attachToParent = function(parent)
         {
-            parent$.__enclos_env__$private$children <- c(parent$.__enclos_env__$private$children, self)
-            private$parent <- parent
+            if(private$name != "")
+            {
+                parent$.__enclos_env__$private$children <- c(parent$.__enclos_env__$private$children, self)
+                private$parent <- parent
+            }
         }
     ),
     

diff --git a/sdk/R/README b/sdk/R/README
index 560be87d8a4ea5a961b9e9d686c60389d7a1e8dd..14a0c9b4c667cf10b8d1b7e6160513e6b6287e32 100644 (file)
--- a/sdk/R/README
+++ b/sdk/R/README
@@ -73,7 +73,9 @@ arvadosSubcollection <- collection$get("location/to/my/directory/")
 
 #Read whole file or just a portion of it.
 
-fileContent <- arvadosFile$read(offset = 1024, length = 512)
+fileContent <- arvadosFile$read()
+fileContent <- arvadosFile$read("text")
+fileContent <- arvadosFile$read("raw", offset = 1024, length = 512)
 
 --------------------------------------------------------------------------------------------------------------------------------