Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix path concat in HTTPStore #42

Merged
merged 3 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions docs/getting-started/remote-data.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
});
```
14 changes: 8 additions & 6 deletions src/storage/httpStore.ts
Original file line number Diff line number Diff line change
@@ -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> {
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('/');
}
13 changes: 10 additions & 3 deletions test/storage/httpStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});
});
12 changes: 12 additions & 0 deletions test/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});