Pass in fs into LdpFileStore

dmitrizagidulin · dmitrizagidulin · commit 127dde13b95f · 2018-10-04T20:29:40.000-04:00
diff --git a/lib/api/ldp/api.js b/lib/api/ldp/api.js
@@ -9,10 +9,21 @@ class LdpHttpHandler {
   /**
    * Usage:
    *   ```
-   *   const store = new LdpFileStore({host, fetch, ...})
+   *   const mapperOptions = {
+   *     rootUrl: host.serverUri,
+   *     rootPath: host.root,
+   *     includeHost: host.multiuser
+   *   }
+   *   const mapper = new LegacyResourceMapper(mapperOptions)
+   *
+   *   const storeOptions = {
+   *     host, mapper, suffixMeta, suffixAcl, dataBrowserPath, suppressDataBrowser
+   *   }
+   *   const store = new LdpFileStore(storeOptions)
+   *
    *   const acl = new AclChecker({host, fetch, store, ...})
    *
-   *   const ldp = new LdpHttpHandler({store, acl})
+   *   const ldp = new LdpHttpHandler({store, acl, host, fetch})
    *   ```
    *
    * @param options.store {LdpFileStore|LdpMemoryStore|LdpQuadStore} storage backend
diff --git a/lib/api/ldp/ldp-operation.js b/lib/api/ldp/ldp-operation.js
@@ -56,8 +56,10 @@ class LdpOperation {
 
   writeCommonHeaders ({res, result}) {
     // set headers in common to all LDP responses
-    res.set('Content-Type', result.contentType)
-    res.set('Content-Length', result.contentLength)
+    if (result) { // delete operation don't return a result, for example
+      res.set('Content-Type', result.contentType)
+      res.set('Content-Length', result.contentLength)
+    }
   }
 }
 
@@ -145,8 +147,8 @@ class LdpGlobOperation extends LdpGetOperation {}
  *
  * If target is a resource:
  *   - writes the resource, always overwriting existing contents if any existed
- *     (since we don't support conditional `If-None-Match` requests)
- *   - has `mkdir -p` semantics (creates missing container hierarchy)
+ *     (since we currently don't support conditional `If-None-Match` requests)
+ *   - has `mkdir -p` semantics (creates intermediate container hierarchy)
  */
 class LdpPutOperation extends LdpOperation {}
 
diff --git a/lib/storage/ldp/ldp-file-store.js b/lib/storage/ldp/ldp-file-store.js
@@ -1,28 +1,25 @@
 'use strict'
 
-const fs = require('fs-extra')
 const { LdpFileResource, LdpFileContainer } = require('./ldp-file-resource')
 
 class LdpFileStore {
   /**
    * @param options {object}
    *
-   * @param options.root {string} Root fs path for data storage
-   * @param options.multiuser {boolean}
+   * @param options.fs {object} Expects the `fs-extra` API
+   * @param options.mapper {LegacyResourceMapper}
+   *
    * @param options.suffixAcl {string}
    * @param options.suffixMeta {string}
-   * @param options.mapper {LegacyResourceMapper}
    * @param options.dataBrowserPath {string}
-   * @param options.suppressDataBrowser
+   * @param options.suppressDataBrowser {boolean}
    */
   constructor (options) {
-    this.root = options.root
-    this.multiuser = options.multiuser
-    this.suffixAcl = options.suffixAcl
-    this.suffixMeta = options.suffixMeta
-
+    this.fs = options.fs
     this.mapper = options.mapper
 
+    this.suffixAcl = options.suffixAcl
+    this.suffixMeta = options.suffixMeta
     this.dataBrowserPath = options.dataBrowserPath
     this.suppressDataBrowser = options.suppressDataBrowser
   }
@@ -34,7 +31,7 @@ class LdpFileStore {
    * @throws {Error} When encountering a filesystem error that's not "File does
    *   not exist", such as `EACCES` etc.
    *
-   * @see https://nodejs.org/api/fs.html#fs_class_fs_stats
+   * @see https://nodejs.org/api/this.fs.html#fs_class_fs_stats
    *
    * @returns {Promise<LdpFileResource|LdpFileContainer>}
    */
@@ -46,7 +43,7 @@ class LdpFileStore {
 
     // Try and load file metadata
     try {
-      fsStats = await fs.stat(path)
+      fsStats = await this.fs.stat(path)
     } catch (error) {
       if (error.code === 'ENOENT') {
         exists = false
@@ -55,12 +52,7 @@ class LdpFileStore {
       }
     }
 
-    let isContainer
-    if (exists) {
-      isContainer = fsStats.isDirectory()
-    } else {
-      isContainer = target.url.endsWith('/')
-    }
+    const isContainer = fsStats ? fsStats.isDirectory() : target.url.endsWith('/')
 
     const encoding = target.charset() // todo: add default charset 'utf8'?
 
@@ -77,7 +69,7 @@ class LdpFileStore {
   async exists (target) {
     const resource = await this.resource(target)
 
-    return fs.pathExists(resource.filePath)
+    return this.fs.pathExists(resource.filePath)
   }
 
   /**
@@ -107,7 +99,7 @@ class LdpFileStore {
    * @returns {Promise}
    */
   async loadContainerContents (container) {
-    container.contents = await fs.readdir(container.path, container.encoding)
+    container.contents = await this.fs.readdir(container.path, container.encoding)
   }
 
   /**
@@ -116,7 +108,7 @@ class LdpFileStore {
    * @returns {Promise}
    */
   async deleteResource (resource) {
-    return fs.remove(resource.path)
+    return this.fs.remove(resource.path)
   }
 
   /**
@@ -130,7 +122,7 @@ class LdpFileStore {
    * @returns {Promise}
    */
   async deleteContainer (container) {
-    return fs.remove(container.path)
+    return this.fs.remove(container.path)
   }
 }
 
