Fixes for dispatch in Blob open() methods

The dispatch here was confusing. Refactor this so that opening a Blob
with various types dispatches down to a method involving the data root
resource controller and path relative to that.

Author: c42f

src/BlobTree.jl

@@ -19,6 +19,7 @@ abstract type AbstractBlobTree; end
 # TODO: Should we have `istree` separate from `isdir`?
 Base.isdir(x::AbstractBlobTree) = true
 Base.isfile(tree::AbstractBlobTree) = false
+Base.ispath(x::AbstractBlobTree) = true
 
 # Number of children is not known without a (potentially high-latency) call to
 # an external resource
@@ -125,27 +126,62 @@ Base.basename(file::Blob) = basename(file.path)
 Base.abspath(file::Blob) = AbsPath(file.root, file.path)
 Base.isdir(file::Blob) = false
 Base.isfile(file::Blob) = true
+Base.ispath(file::Blob) = true
 
 function Base.show(io::IO, ::MIME"text/plain", file::Blob)
-    print(io, "📄 ", file.path, " @ ", sys_abspath(file.root))
+    print(io, "📄 ", file.path, " @ ", summary(file.root))
 end
 
 function AbstractTrees.printnode(io::IO, file::Blob)
     print(io, "📄 ",  basename(file))
 end
 
+# Opening as Vector{UInt8} or as String uses IO interface
 function Base.open(f::Function, ::Type{Vector{UInt8}}, file::Blob)
-    open(IO, file) do io
+    open(IO, file.root, file.path) do io
         f(read(io)) # TODO: use Mmap?
     end
 end
 
 function Base.open(f::Function, ::Type{String}, file::Blob)
-    open(Vector{UInt8}, file) do buf
-        f(String(buf))
+    open(IO, file.root, file.path) do io
+        f(read(io, String))
     end
 end
 
+# Default open-type for Blob is IO
+Base.open(f::Function, file::Blob; kws...) = open(f, IO, file.root, file.path; kws...)
+
+# Opening Blob as itself is trivial
+function Base.open(f::Function, ::Type{Blob}, file::Blob)
+    f(file)
+end
+
+# open with other types T defers to the underlying storage system
+function Base.open(f::Function, ::Type{T}, file::Blob; kws...) where {T}
+    open(f, T, file.root, file.path; kws...)
+end
+
+# Unscoped form of open
+function Base.open(::Type{T}, file::Blob; kws...) where {T}
+    open(identity, T, file; kws...)
+end
+
+# read() is also supported for `Blob`s
+Base.read(file::Blob) = read(file.root, file.path)
+Base.read(file::Blob, ::Type{T}) where {T} = read(file.root, file.path, T)
+
+
+# Support for opening AbsPath
+#
+# TODO: Put this elsewhere?
+function Base.open(f::Function, ::Type{T}, path::AbsPath; kws...) where {T}
+    open(f, T, path.root, path.path; kws...)
+end
+
+Base.open(f::Function, path::AbsPath; kws...) = open(f, IO, path.root, path.path; kws...)
+
+
 #-------------------------------------------------------------------------------
 struct BlobTree{Root} <: AbstractBlobTree
     root::Root
@@ -165,7 +201,7 @@ function Base.show(io::IO, ::MIME"text/plain", tree::AbstractBlobTree)
     # `children()` for all our trees. It'd be much easier if
     # `AbstractTrees.has_children()` was used consistently upstream.
     cs = children(tree)
-    println(io, "📂 Tree ", tree.path, " @ ", tree.root)
+    println(io, "📂 Tree ", tree.path, " @ ", summary(tree.root))
     for (i, c) in enumerate(cs)
         print(io, " ", isdir(c) ? '📁' : '📄', " ", basename(c))
         if i != length(cs)
@@ -228,15 +264,8 @@ function children(tree::BlobTree)
     [tree[c] for c in child_names]
 end
 
-Base.open(f::Function, file::Blob; kws...) = open(f, file.root, file.path; kws...)
-Base.open(f::Function, path::AbsPath; kws...) = open(f, path.root, path.path; kws...)
-
 function Base.open(f::Function, ::Type{BlobTree}, tree::BlobTree)
     f(tree)
 end
 
-function Base.open(f::Function, ::Type{Blob}, file::Blob)
-    f(file)
-end
-
 # Base.open(::Type{T}, file::Blob; kws...) where {T} = open(identity, T, file.root, file.path; kws...)

src/DataSets.jl

@@ -270,6 +270,11 @@ include("filesystem.jl")
 # opened. See check_scoped_open for a way to help users avoid errors when using
 # this (ie, if `identity` is not a valid argument to open() because resources
 # would be closed before it returns).
+#
+# FIXME: Consider removing this. It should likely be replaced with `load()`, in
+# analogy to FileIO.jl's load operation:
+# * `load()` is "load the entire file into memory as such-and-such type"
+# * `open()` is "open this resource, and run some function while it's open"
 Base.open(as_type, conf::DataSet) = open(identity, as_type, conf)
 
 """

src/filesystem.jl

@@ -21,21 +21,21 @@ sys_abspath(file::Blob) = sys_abspath(file.root, file.path)
 # AbsPath{<:AbstractFileSystemRoot} rather than usin double dispatch?
 Base.isdir(root::AbstractFileSystemRoot, path::RelPath) = isdir(sys_abspath(root, path))
 Base.isfile(root::AbstractFileSystemRoot, path::RelPath) = isfile(sys_abspath(root, path))
+Base.ispath(root::AbstractFileSystemRoot, path::RelPath) = ispath(sys_abspath(root, path))
 Base.read(root::AbstractFileSystemRoot, path::RelPath, ::Type{T}) where {T} =
     read(sys_abspath(root, path), T)
+Base.read(root::AbstractFileSystemRoot, path::RelPath) where {T} =
+    read(sys_abspath(root, path))
 
-# TODO: Is it possible to get a generic version of this without type piracy?
-function Base.open(::Type{T}, file::Blob{<:AbstractFileSystemRoot}; kws...) where {T}
-    open(identity, T, file)
-end
+Base.summary(io::IO, root::AbstractFileSystemRoot) = print(io, sys_abspath(root))
 
-function Base.open(f::Function, ::Type{IO}, file::Blob{<:AbstractFileSystemRoot};
+function Base.open(f::Function, ::Type{IO}, root::AbstractFileSystemRoot, path;
                    write=false, read=!write, kws...)
-    if !iswriteable(file.root) && write
+    if !iswriteable(root) && write
         error("Error writing file at read-only path $path")
     end
     check_scoped_open(f, IO)
-    open(f, sys_abspath(file.root, file.path); read=read, write=write, kws...)
+    open(f, sys_abspath(root, path); read=read, write=write, kws...)
 end
 
 function Base.mkdir(root::AbstractFileSystemRoot, path::RelPath; kws...)