Merge pull request #42 from gzuidhof/gzuidhof.fix-url-concat-in-https…

…tore

Fix path concat in HTTPStore

docs/getting-started/remote-data.md

@@ -181,17 +181,3 @@ NumPy-like [broadcasting](https://docs.scipy.org/doc/numpy/user/basics.broadcast
 ## Known issues
 
 * Open ended slicing with negative indices seems to have a bug. For example slice `x.get(slice(-5))` does not give the same results as `x[-5:]` in NumPy. *(contributions are welcome!)*
-* Reading data from a remote store should be possible like this:
-    ```javascript
-    const z = await openArray({
-        store: "http://localhost:8000/dummy_dataset.zarr",
-    });
-    ```
-    But because of the way we do URL concatenation this doesn't work.. should be an easy fix in the HTTPStore file.  
-    The workaround for now:
-    ```javascript
-    const z = await openArray({
-        store: "http://localhost:8000/",
-        path: "dummy_dataset.zarr",
-    });
-    ```

src/storage/httpStore.ts

@@ -1,5 +1,5 @@
 import { ValidStoreType, AsyncStore } from './types';
-import { IS_NODE } from '../util';
+import { IS_NODE, joinUrlParts } from '../util';
 import { KeyError, HTTPError } from '../errors';
 
 export class HTTPStore implements AsyncStore<ArrayBuffer> {
@@ -19,7 +19,7 @@ export class HTTPStore implements AsyncStore<ArrayBuffer> {
     }
 
     async getItem(item: string) {
-        const url = new URL(item, this.url).href;
+        const url = joinUrlParts(this.url, item);
         const value = await fetch(url);
 
         if (value.status === 404) {
@@ -28,27 +28,29 @@ export class HTTPStore implements AsyncStore<ArrayBuffer> {
         } else if (value.status !== 200) {
             throw new HTTPError(String(value.status));
         }
+
         // only decode if 200
         if (IS_NODE) {
-            // Node
             return Buffer.from(await value.arrayBuffer());
+        } else {
+            return value.arrayBuffer(); // Browser
         }
-        return value.arrayBuffer(); // Browser
     }
 
     async setItem(item: string, value: ValidStoreType): Promise<boolean> {
+        const url = joinUrlParts(this.url, item);
         if (typeof value === 'string') {
             value = new TextEncoder().encode(value).buffer;
         }
-        const set = await fetch(item, { method: 'PUT' });
+        const set = await fetch(url, { method: 'PUT', body: value});
         return set.status.toString()[0] === '2';
     }
 
     deleteItem(_item: string): Promise<boolean> {
         throw new Error('Method not implemented.');
     }
     async containsItem(item: string): Promise<boolean> {
-        const url = new URL(item, this.url).href;
+        const url = joinUrlParts(this.url, item);
         const value = await fetch(url);
 
         return value.status === 200;

src/util.ts

@@ -200,3 +200,18 @@ export function getStrides(shape: number[]): number[] {
     }
     return strides;
 }
+
+/**
+ * Preserves (double) slashes earlier in the path, so this works better
+ * for URLs. From https://stackoverflow.com/a/46427607/4178400
+ * @param args parts of a path or URL to join.
+ */
+export function joinUrlParts(...args: string[]) {
+    return args.map((part, i) => {
+        if (i === 0) {
+          return part.trim().replace(/[\/]*$/g, '');
+        } else {
+          return part.trim().replace(/(^[\/]*|[\/]*$)/g, '');
+        }
+      }).filter(x=>x.length).join('/');
+}

test/storage/httpStore.test.ts

@@ -4,23 +4,30 @@ import { HTTPStore } from "../../src/storage/httpStore";
 import { openArray } from "../../src/creation";
 
 describe("Test MemoryStore", () => {
-    const hStore = new HTTPStore("http://localhost:7357/");
 
+    const simpleFixtureStore = new HTTPStore("http://localhost:7357/simple.zarr");
     it("Can open simple fixture", async () => {
-        const z = await openArray({ store: hStore, path: "simple.zarr" });
+        const z = await openArray({ store: simpleFixtureStore});
         expect(z.shape).toEqual([8, 8]);
         expect(await z.get([0, 0])).toEqual(1);
         expect(await z.get([0, 1])).toEqual(2);
         expect(await z.get([7, 7])).toEqual(3);
         expect(await z.get([4, 4])).toEqual(0);
     });
 
+    const emptyFixtureStore = new HTTPStore("http://localhost:7357/empty.zarr");
     it("Can open empty fixture", async () => {
-        const z = await openArray({ store: hStore, path: "empty.zarr" });
+        const z = await openArray({ store: emptyFixtureStore});
         expect(z.shape).toEqual([8, 8]);
         expect(await z.get([0, 0])).toEqual(0);
         expect(await z.get([0, 1])).toEqual(0);
         expect(await z.get([7, 7])).toEqual(0);
         expect(await z.get([4, 4])).toEqual(0);
     });
+
+    const baseUrlStore = new HTTPStore("http://localhost:7357");
+    it("Can open by path", async() => {
+        const z = await openArray({ store: baseUrlStore, path: "simple.zarr"});
+        expect(z.shape).toEqual([8, 8]);
+    });
 });

test/util.test.ts

@@ -74,4 +74,16 @@ describe("ArrayEquals1D works", () => {
     ])("1D Array should not be equal", (arrA, arrB) => {
         expect(util.arrayEquals1D(arrA, arrB)).toBeFalsy();
     });
+});
+
+describe("URL joining works", () => {
+    test.each([
+        [["https://example.com", "bla"], "https://example.com/bla"],
+        [["https://example.com/my-store", "arr.zarr"], "https://example.com/my-store/arr.zarr"],
+        [["https://example.com/", "arr.zarr"], "https://example.com/arr.zarr"],
+        [["https://example.com/", "", "arr.zarr"], "https://example.com/arr.zarr"],
+    // eslint-disable-next-line @typescript-eslint/ban-types
+    ])("joins parts as expected: output %s, expected %p", (parts: string[] | String, expected: string) => {
+        expect(util.joinUrlParts(...parts)).toEqual(expected);
+    });
 });