Collection classes no longer use each others private state.
authorFuad Muhic <fmuhic@capeannenterprises.com>
Tue, 9 Jan 2018 14:28:42 +0000 (15:28 +0100)
committerFuad Muhic <fmuhic@capeannenterprises.com>
Tue, 9 Jan 2018 14:28:42 +0000 (15:28 +0100)
Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuhic@capeannenterprises.com>

sdk/R/R/ArvadosFile.R
sdk/R/R/Collection.R
sdk/R/R/CollectionTree.R
sdk/R/R/HttpRequest.R
sdk/R/R/Subcollection.R

index 3be24c464f918b13b371256089aa98fa4d046671..8c2e9e2547d2c071a8515501555324a4c82eb780 100644 (file)
@@ -37,17 +37,21 @@ ArvadosFile <- R6::R6Class(
             as.numeric(sizes)
         },
 
-        removeFromCollection = function()
+        get = function(fileLikeObjectName)
         {
-            if(is.null(private$collection))
-                stop("ArvadosFile doesn't belong to any collection.")
-            
-            private$collection$.__enclos_env__$private$deleteFromREST(self$getRelativePath())
+            return(NULL)
+        },
 
-            private$addToCollection(NULL)
-            private$detachFromParent()
+        getFirst = function()
+        {
+            return(NULL)
+        },
 
-            "Content removed successfully."
+        getCollection = function() private$collection,
+
+        setCollection = function(collection)
+        {
+            private$collection <- collection
         },
 
         getRelativePath = function()
@@ -67,6 +71,8 @@ ArvadosFile <- R6::R6Class(
 
         getParent = function() private$parent,
 
+        setParent = function(newParent) private$parent <- newParent,
+
         read = function(contentType = "raw", offset = 0, length = 0)
         {
             if(is.null(private$collection))
@@ -128,17 +134,21 @@ ArvadosFile <- R6::R6Class(
 
         move = function(newLocation)
         {
+            #todo test if file can be moved
+
             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, "/")))
+                                      nchar(newLocation)
+                                      - nchar(paste0(private$name, "/")))
             }
             else if(endsWith(newLocation, private$name))
             {
-                newLocation <- substr(newLocation, 0, nchar(newLocation) - nchar(private$name))
+                newLocation <- substr(newLocation, 0,
+                                      nchar(newLocation) - nchar(private$name))
             }
             else
             {
@@ -152,10 +162,25 @@ ArvadosFile <- R6::R6Class(
                 stop("Unable to get destination subcollection.")
             }
 
-            status <- private$collection$.__enclos_env__$private$moveOnREST(self$getRelativePath(),
-                                                                            paste0(newParent$getRelativePath(), "/", self$getName()))
+            childWithSameName <- newParent$get(private$name)
+
+            if(!is.null(childWithSameName))
+                stop("Destination already contains file with same name.")
 
-            private$attachToParent(newParent)
+            status <- private$collection$moveOnREST(self$getRelativePath(),
+                                                    paste0(newParent$getRelativePath(),
+                                                           "/", self$getName()))
+
+            #Note: We temporary set parents collection to NULL. This will ensure that
+            #      add method doesn't post file on REST server.
+            parentsCollection <- newParent$getCollection()
+            newParent$setCollection(NULL, setRecursively = FALSE)
+
+            newParent$add(self)
+
+            newParent$setCollection(parentsCollection, setRecursively = FALSE)
+
+            private$parent <- newParent
 
             "Content moved successfully."
         }
@@ -168,37 +193,7 @@ ArvadosFile <- R6::R6Class(
         parent     = NULL,
         collection = NULL,
         http       = NULL,
-        httpParser = NULL,
-
-        getChild = function(name)
-        {
-            return(NULL)
-        },
-
-        getFirstChild = function()
-        {
-            return(NULL)
-        },
-
-        addToCollection = function(collection)
-        {
-            private$collection <- collection
-        },
-
-        detachFromParent = function()
-        {
-            if(!is.null(private$parent))
-            {
-                private$parent$.__enclos_env__$private$removeChild(private$name)
-                private$parent <- NULL
-            }
-        },
-
-        attachToParent = function(parent)
-        {
-            parent$.__enclos_env__$private$children <- c(parent$.__enclos_env__$private$children, self)
-            private$parent <- parent
-        }
+        httpParser = NULL
     ),
     
     cloneable = FALSE
index 8e6895e823ae46c023bdd25801a1bc7a47ea9af4..8bd4655c2bea391ee26cf40df7d12cd959ab9bef 100644 (file)
@@ -37,7 +37,7 @@ Collection <- R6::R6Class(
                relativePath == "." ||
                relativePath == "./")
             {
-                subcollection <- private$tree$.__enclos_env__$private$tree
+                subcollection <- private$tree$getTree()
             }
             else
             {
@@ -71,7 +71,7 @@ Collection <- R6::R6Class(
                relativePath == "." ||
                relativePath == "./")
             {
-                subcollection <- private$tree$.__enclos_env__$private$tree
+                subcollection <- private$tree$getTree()
             }
             else
             {
@@ -89,6 +89,10 @@ Collection <- R6::R6Class(
                 arvadosFiles <- NULL
                 sapply(fileNames, function(fileName)
                 {
+                    childWithSameName <- subcollection$get(fileName)
+                    if(!is.null(childWithSameName))
+                        stop("Destination already contains file with same name.")
+
                     newFile <- ArvadosFile$new(fileName)
                     subcollection$add(newFile)
 
@@ -122,14 +126,15 @@ Collection <- R6::R6Class(
                     if(is.null(file))
                         stop(paste("File", filePath, "doesn't exist."))
 
-                    file$removeFromCollection()
+                    parent <- file$getParent()
+                    parent$remove(filePath)
                 })
             }
             else if("ArvadosFile"   %in% class(content) ||
                     "Subcollection" %in% class(content))
             {
-                if(is.null(content$.__enclos_env__$private$collection) || 
-                   content$.__enclos_env__$private$collection$uuid != self$uuid)
+                if(is.null(content$getCollection()) || 
+                   content$getCollection()$uuid != self$uuid)
                     stop("Subcollection doesn't belong to this collection.")
 
                 content$removeFromCollection()
@@ -154,44 +159,14 @@ Collection <- R6::R6Class(
         get = function(relativePath)
         {
             private$tree$getElement(relativePath)
-        }
-    ),
-
-    private = list(
-
-        http       = NULL,
-        httpParser = NULL,
-        tree       = NULL,
-
-        fileContent = NULL,
-
-        getCollectionContent = function()
-        {
-            collectionURL <- URLencode(paste0(self$api$getWebDavHostName(), "c=", self$uuid))
-
-            headers = list("Authorization" = paste("OAuth2", self$api$getToken()))
-
-            response <- private$http$PROPFIND(collectionURL, headers)
-
-            parsedResponse <- private$httpParser$parseWebDAVResponse(response, collectionURL)
-            parsedResponse[-1]
         },
-
+        
+        #Todo: Move these methods to another class.
         createFilesOnREST = function(files)
         {
             sapply(files, function(filePath)
             {
-                private$createNewFile(filePath, NULL, "text/html")
-            })
-        },
-        
-        generateTree = function(content)
-        {
-            treeBranches <- sapply(collectionContent, function(filePath)
-            {
-                splitPath <- unlist(strsplit(filePath$name, "/", fixed = TRUE))
-
-                branch = private$createBranch(splitPath, filePath$fileSize)      
+                self$createNewFile(filePath, NULL, "text/html")
             })
         },
 
@@ -209,7 +184,7 @@ Collection <- R6::R6Class(
 
             print(paste("File created:", relativePath))
         },
-        
+
         deleteFromREST = function(relativePath)
         {
             fileURL <- paste0(self$api$getWebDavHostName(), "c=", self$uuid, "/", relativePath);
@@ -241,5 +216,36 @@ Collection <- R6::R6Class(
         }
     ),
 
+    private = list(
+
+        http       = NULL,
+        httpParser = NULL,
+        tree       = NULL,
+
+        fileContent = NULL,
+
+        getCollectionContent = function()
+        {
+            collectionURL <- URLencode(paste0(self$api$getWebDavHostName(), "c=", self$uuid))
+
+            headers = list("Authorization" = paste("OAuth2", self$api$getToken()))
+
+            response <- private$http$PROPFIND(collectionURL, headers)
+
+            parsedResponse <- private$httpParser$parseWebDAVResponse(response, collectionURL)
+            parsedResponse[-1]
+        },
+
+        generateTree = function(content)
+        {
+            treeBranches <- sapply(collectionContent, function(filePath)
+            {
+                splitPath <- unlist(strsplit(filePath$name, "/", fixed = TRUE))
+
+                branch = private$createBranch(splitPath, filePath$fileSize)      
+            })
+        }
+    ),
+
     cloneable = FALSE
 )
index 3bf89e5f5a3bd48eeb23e5283fd66512068f2aca..e024f5eb0f1802703d267b04693dd905876af081 100644 (file)
@@ -21,7 +21,7 @@ CollectionTree <- R6::R6Class(
             treeBranches <- sapply(fileContent, function(filePath)
             {
                 splitPath <- unlist(strsplit(filePath, "/", fixed = TRUE))
-                branch = private$createBranch(splitPath)      
+                branch <- private$createBranch(splitPath)      
             })
 
             root <- Subcollection$new("")
@@ -31,7 +31,7 @@ CollectionTree <- R6::R6Class(
                 private$addBranch(root, branch)
             })
 
-            root$.__enclos_env__$private$addToCollection(collection)
+            root$setCollection(collection)
             private$tree <- root
         },
 
@@ -41,18 +41,20 @@ CollectionTree <- R6::R6Class(
                 relativePath <- substr(relativePath, 0, nchar(relativePath) - 1)
 
             splitPath <- unlist(strsplit(relativePath, "/", fixed = TRUE))
-            returnElement = private$tree
+            returnElement <- private$tree
 
             for(pathFragment in splitPath)
             {
-                returnElement = returnElement$.__enclos_env__$private$getChild(pathFragment)
+                returnElement <- returnElement$get(pathFragment)
 
                 if(is.null(returnElement))
                     return(NULL)
             }
 
             returnElement
-        }
+        },
+
+        getTree = function() private$tree
     ),
 
     private = list(
@@ -68,13 +70,13 @@ CollectionTree <- R6::R6Class(
             {
                 if(elementIndex == lastElementIndex)
                 {
-                    branch = ArvadosFile$new(splitPath[[elementIndex]])
+                    branch <- ArvadosFile$new(splitPath[[elementIndex]])
                 }
                 else
                 {
-                    newFolder = Subcollection$new(splitPath[[elementIndex]])
+                    newFolder <- Subcollection$new(splitPath[[elementIndex]])
                     newFolder$add(branch)
-                    branch = newFolder
+                    branch <- newFolder
                 }
             }
             
@@ -83,7 +85,7 @@ CollectionTree <- R6::R6Class(
 
         addBranch = function(container, node)
         {
-            child = container$.__enclos_env__$private$getChild(node$getName())
+            child <- container$get(node$getName())
 
             if(is.null(child))
             {
@@ -96,18 +98,18 @@ CollectionTree <- R6::R6Class(
                     child = private$replaceFileWithSubcollection(child)
                 }
 
-                private$addBranch(child, node$.__enclos_env__$private$getFirstChild())
+                private$addBranch(child, node$getFirst())
             }
         },
 
         replaceFileWithSubcollection = function(arvadosFile)
         {
             subcollection <- Subcollection$new(arvadosFile$getName())
-            fileParent <- arvadosFile$.__enclos_env__$private$parent
-            fileParent$.__enclos_env__$private$removeChild(arvadosFile$getName())
+            fileParent <- arvadosFile$getParent()
+            fileParent$remove(arvadosFile$getName())
             fileParent$add(subcollection)
 
-            arvadosFile$.__enclos_env__$private$parent <- NULL
+            arvadosFile$setParent(NULL)
 
             subcollection
         }
index a1e1f61275263ec63910f5d48a5d017621fb02ff..ad153a044caf858748a30516658ae901cf229cef 100644 (file)
@@ -21,17 +21,18 @@ HttpRequest <- R6::R6Class(
         },
 
         PUT = function(url, headers = NULL, body = NULL,
-                       queryFilters = NULL, limit = 100, offset = 0)
+                       queryFilters = NULL, limit = NULL, offset = NULL)
         {
             headers <- httr::add_headers(unlist(headers))
             query <- private$createQuery(queryFilters, limit, offset)
             url <- paste0(url, query)
+            print(url)
 
             serverResponse <- httr::PUT(url = url, config = headers, body = body)
         },
 
         POST = function(url, headers = NULL, body = NULL,
-                        queryFilters = NULL, limit = 100, offset = 0)
+                        queryFilters = NULL, limit = NULL, offset = NULL)
         {
             headers <- httr::add_headers(unlist(headers))
             query <- private$createQuery(queryFilters, limit, offset)
@@ -141,7 +142,8 @@ HttpRequest <- R6::R6Class(
                 finalQuery <- paste0(finalQuery, collapse = "&")
             }
 
-            finalQuery <- paste0("/?", finalQuery)
+            if(!is.null(finalQuery))
+                finalQuery <- paste0("/?", finalQuery)
 
             finalQuery
         }
index 296cb92e5ec255220cc563354f29b674dabe0df7..06df7c48767280f601dcbd30ceec9558adfccdd5 100644 (file)
@@ -15,31 +15,48 @@ Subcollection <- R6::R6Class(
             private$http       <- HttpRequest$new()
             private$httpParser <- HttpParser$new()
         },
+
+        getName = function() private$name,
         
+        getRelativePath = function()
+        {
+            relativePath <- c(private$name)
+            parent <- private$parent
+
+            while(!is.null(parent))
+            {
+                relativePath <- c(parent$getName(), relativePath)
+                parent <- parent$getParent()
+            }
+
+            relativePath <- relativePath[relativePath != ""]
+            paste0(relativePath, collapse = "/")
+        },
+
         add = function(content)
         {
             if("ArvadosFile"   %in% class(content) ||
                "Subcollection" %in% class(content))
             {
-                if(!is.null(content$.__enclos_env__$private$collection))
-                    stop("ArvadosFile/Subcollection already belongs to a collection.")
-
-                childWithSameName <- private$getChild(content$getName())
+                childWithSameName <- self$get(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$getFileListing())
-
-                    private$collection$.__enclos_env__$private$createFilesOnREST(contentPath)
-                    content$.__enclos_env__$private$addToCollection(private$collection)
+                    if(self$getRelativePath() != "")
+                        contentPath <- paste0(self$getRelativePath(),
+                                              "/", content$getFileListing())
+                    else
+                        contentPath <- content$getFileListing()
+
+                    private$collection$createFilesOnREST(contentPath)
+                    content$setCollection(private$collection)
                 }
 
                 private$children <- c(private$children, content)
-                content$.__enclos_env__$private$parent = self
+                content$setParent(self)
 
                 "Content added successfully."
             }
@@ -50,25 +67,31 @@ Subcollection <- R6::R6Class(
             }
         },
 
-        removeFromCollection = function()
+        remove = function(name)
         {
-            if(is.null(private$collection))
-                stop("Subcollection doesn't belong to any collection.")
+            if(is.character(name))
+            {
+                child <- self$get(name)
 
-            if(private$name == "")
-                stop("Unable to delete root folder.")
+                if(is.null(child))
+                    stop("Subcollection doesn't contains ArvadosFile
+                          or Subcollection with same name.")
 
-            collectionList <- paste0(self$getRelativePath(),
-                                     "/", self$getFileListing(fullpath = FALSE))
-            sapply(collectionList, function(file)
-            {
-                private$collection$.__enclos_env__$private$deleteFromREST(file)
-            })
+                if(!is.null(private$collection))
+                {
+                    private$collection$deleteFromREST(child$getRelativePath())
+                    child$setCollection(NULL)
+                }
 
-            private$addToCollection(NULL)
-            private$dettachFromParent()
+                private$removeChild(name)
+                child$setParent(NULL)
 
-            "Content removed successfully."
+                "Content removed"
+            }
+            else
+            {
+                stop(paste("Expected character, got", class(content), "."))
+            }
         },
 
         getFileListing = function(fullpath = TRUE)
@@ -108,23 +131,6 @@ Subcollection <- R6::R6Class(
             sum(sizes)
         },
 
-        getName = function() private$name,
-
-        getRelativePath = function()
-        {
-            relativePath <- c(private$name)
-            parent <- private$parent
-
-            while(!is.null(parent))
-            {
-                relativePath <- c(parent$getName(), relativePath)
-                parent <- parent$getParent()
-            }
-
-            relativePath <- relativePath[relativePath != ""]
-            paste0(relativePath, collapse = "/")
-        },
-
         move = function(newLocation)
         {
             if(is.null(private$collection))
@@ -152,27 +158,25 @@ Subcollection <- R6::R6Class(
                 stop("Unable to get destination subcollection.")
             }
 
-            status <- private$collection$.__enclos_env__$private$moveOnREST(self$getRelativePath(),
-                                                                            paste0(newParent$getRelativePath(), "/", self$getName()))
+            status <- private$collection$moveOnREST(self$getRelativePath(),
+                                                    paste0(newParent$getRelativePath(),
+                                                           "/", self$getName()))
 
-            private$attachToParent(newParent)
+            #Note: We temporary set parents collection to NULL. This will ensure that
+            #      add method doesn't post file on REST server.
+            parentsCollection <- newParent$getCollection()
+            newParent$setCollection(NULL, setRecursively = FALSE)
 
-            "Content moved successfully."
-        },
+            newParent$add(self)
 
-        getParent = function() private$parent
-    ),
+            newParent$setCollection(parentsCollection, setRecursively = FALSE)
 
-    private = list(
+            private$parent <- newParent
 
-        name       = NULL,
-        children   = NULL,
-        parent     = NULL,
-        collection = NULL,
-        http       = NULL,
-        httpParser = NULL,
+            "Content moved successfully."
+        },
 
-        getChild = function(name)
+        get = function(name)
         {
             for(child in private$children)
             {
@@ -183,7 +187,7 @@ Subcollection <- R6::R6Class(
             return(NULL)
         },
 
-        getFirstChild = function()
+        getFirst = function()
         {
             if(length(private$children) == 0)
                return(NULL)
@@ -191,6 +195,33 @@ Subcollection <- R6::R6Class(
             private$children[[1]]
         },
 
+        setCollection = function(collection, setRecursively = TRUE)
+        {
+            private$collection = collection
+
+            if(setRecursively)
+            {
+                for(child in private$children)
+                    child$setCollection(collection)
+            }
+        },
+
+        getCollection = function() private$collection,
+
+        getParent = function() private$parent,
+
+        setParent = function(newParent) private$parent <- newParent
+    ),
+
+    private = list(
+
+        name       = NULL,
+        children   = NULL,
+        parent     = NULL,
+        collection = NULL,
+        http       = NULL,
+        httpParser = NULL,
+
         removeChild = function(name)
         {
             numberOfChildren = length(private$children)
@@ -205,34 +236,6 @@ Subcollection <- R6::R6Class(
                     }
                 }
             }
-        },
-
-        addToCollection = function(collection)
-        {
-            for(child in private$children)
-                child$.__enclos_env__$private$addToCollection(collection)
-
-            private$collection = collection
-        },
-
-        dettachFromParent = function()
-        {
-            if(!is.null(private$parent))
-            {
-                private$parent$.__enclos_env__$private$removeChild(private$name)
-                private$parent <- NULL
-            }
-            else
-                stop("Parent doesn't exists.")
-        },
-
-        attachToParent = function(parent)
-        {
-            if(private$name != "")
-            {
-                parent$.__enclos_env__$private$children <- c(parent$.__enclos_env__$private$children, self)
-                private$parent <- parent
-            }
         }
     ),