Skip to content

Commit

Permalink
fix(core): don't override baseUrl when importing views using a url
Browse files Browse the repository at this point in the history
If the spec being imported has a baseUrl, it is now appended (unless absolute) to the
url (path) of the spec being imported.

Also gets rid of vega-loader's loader functions and replaces them with plain fetches.
  • Loading branch information
tuner committed Mar 22, 2024
1 parent 18c7d67 commit d08bce8
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 43 deletions.
5 changes: 4 additions & 1 deletion docs/grammar/import.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@ the imported specification. Thus, you can publish a view (or a track as known in
genome browsers) by placing its specification and data available in the same
directory on a web server.

The URL import supports parameters, which are described below within the
[named templates](#repeating-with-named-templates).

```json title="Example"
{
...,
"vconcat": [
...,
{ "import": { "url": "includes/annotations.json" } },
{ "import": { "url": "https://genomespy.app/tracks/cosmic/census_hg38.json" } }
{ "import": { "url": "https://example.site/tracks/annotations.json" } }
]
}
```
Expand Down
30 changes: 17 additions & 13 deletions packages/core/src/data/sources/urlSource.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { loader as vegaLoader, read } from "vega-loader";
import { read } from "vega-loader";
import { getFormat } from "./dataUtils.js";
import DataSource from "./dataSource.js";
import {
activateExprRefProps,
withoutExprRef,
} from "../../view/paramMediator.js";
import { concatUrl } from "../../utils/url.js";

/**
* @param {Partial<import("../../spec/data.js").Data>} data
Expand Down Expand Up @@ -46,18 +47,21 @@ export default class UrlSource extends DataSource {
}

/** @param {string} url */
const load = async (url) =>
// TODO: Support chunked loading
/** @type {string} */ (
vegaLoader({
baseURL: this.baseUrl,
})
.load(url)
.catch((/** @type {Error} */ e) => {
// TODO: Include baseurl in the error message. Should be normalized, however.
throw new Error(`${url}: ${e.message}`);
})
);
const load = async (url) => {
try {
const fullUrl = concatUrl(this.baseUrl, url);
const result = await fetch(fullUrl);
if (!result.ok) {
throw new Error(`${result.status} ${result.statusText}`);
}
// TODO: what about binary data (for arrow), etc?
return result.text();
} catch (e) {
throw new Error(
`Could not load data: ${url}. Reason: ${e.message}`
);
}
};

/**
* @param {string} text
Expand Down
35 changes: 29 additions & 6 deletions packages/core/src/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,42 @@ const protoRe = /^([A-Za-z]+:)?\/\//;
* Append a relative or absolute url to a base url.
* The base part is omitted if the append part is absolute.
*
* @param {function():string} baseAccessor
* If the base part has no trailing slash, it is assumed to be a file and
* only the directory part is used.
*
* @param {string | (() => string)} base
* @param {string} append
*/
export function appendToBaseUrl(baseAccessor, append) {
export function concatUrl(base, append) {
if (append && protoRe.test(append)) {
return append;
}

const base = baseAccessor();
const baseString = typeof base == "function" ? base() : base;
if (!baseString) {
return append;
}
if (!append) {
return baseString;
}

if (base && append) {
return base.endsWith("/") ? base + append : base + "/" + append;
if (/[#?]/.test(baseString)) {
throw new Error(
`Cannot append to a url with query or hash. Append: ${append}, base: ${baseString}`
);
}

return base ?? append;
return getDirectory(baseString) + append;
}

/**
* @param {string} url
*/
export function getDirectory(url) {
const directory = url.replace(/[^/]*$/, "");
return directory === ""
? undefined
: directory.endsWith("://")
? url + "/"
: directory;
}
28 changes: 28 additions & 0 deletions packages/core/src/utils/url.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { expect, test } from "vitest";
import { concatUrl, getDirectory } from "./url.js";

test("getDirectory", () => {
expect(getDirectory("foo")).toBeUndefined();
expect(getDirectory("foo/")).toBe("foo/");
expect(getDirectory("foo/index")).toBe("foo/");
expect(getDirectory("http://example.com")).toBe("http://example.com/");
expect(getDirectory("http://example.com/")).toBe("http://example.com/");
expect(getDirectory("http://example.com/a")).toBe("http://example.com/");
expect(getDirectory("http://example.com/a/")).toBe("http://example.com/a/");
});

test("concatUrl", () => {
expect(concatUrl("http://example.com", "foo")).toEqual(
"http://example.com/foo"
);
expect(concatUrl(() => "http://example.com", "foo")).toEqual(
"http://example.com/foo"
);
expect(concatUrl("http://example.com/", "http://genomespy.app/")).toEqual(
"http://genomespy.app/"
);
expect(concatUrl("foo/", "bar")).toEqual("foo/bar");
expect(concatUrl("foo/baz", "bar")).toEqual("foo/bar");
expect(concatUrl(undefined, "bar")).toEqual("bar");
expect(concatUrl("bar", undefined)).toEqual("bar");
});
4 changes: 2 additions & 2 deletions packages/core/src/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { isNumber, isString, span } from "vega-util";
import { scaleLog } from "d3-scale";
import { isFieldDef, getPrimaryChannel } from "../encoder/encoder.js";
import { appendToBaseUrl } from "../utils/url.js";
import { concatUrl } from "../utils/url.js";
import { isDiscrete, bandSpace } from "vega-scale";
import { peek } from "../utils/arrayUtils.js";
import ViewError from "./viewError.js";
Expand Down Expand Up @@ -645,7 +645,7 @@ export default class View {
* @returns {string}
*/
getBaseUrl() {
return appendToBaseUrl(
return concatUrl(
() => this.dataParent?.getBaseUrl(),
this.spec.baseUrl
);
Expand Down
51 changes: 30 additions & 21 deletions packages/core/src/view/viewUtils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { isObject, isString } from "vega-util";
import { loader as vegaLoader } from "vega-loader";

import UnitView from "./unitView.js";
// eslint-disable-next-line no-unused-vars
Expand All @@ -8,6 +7,7 @@ import { buildDataFlow } from "./flowBuilder.js";
import { optimizeDataFlow } from "../data/flowOptimizer.js";
import { isFieldDef, primaryPositionalChannels } from "../encoder/encoder.js";
import { rollup } from "d3-array";
import { concatUrl, getDirectory } from "../utils/url.js";

/**
*
Expand Down Expand Up @@ -146,30 +146,39 @@ export function findEncodedFields(view) {
*/
export async function loadExternalViewSpec(spec, baseUrl, viewContext) {
const importParam = spec.import;
if ("url" in importParam) {
const loader = vegaLoader({ baseURL: baseUrl });
const url = importParam.url;
if (!("url" in importParam)) {
throw new Error("Not an url import: " + JSON.stringify(importParam));
}

const importedSpec = JSON.parse(
await loader.load(url).catch((/** @type {Error} */ e) => {
throw new Error(
`Could not load imported view spec: ${url} \nReason: ${e.message}`
);
})
);
const url = concatUrl(baseUrl, importParam.url);

/** @type {import("../spec/view.js").ViewSpec} */
let importedSpec;

if (viewContext.isViewSpec(importedSpec)) {
importedSpec.baseUrl = url.match(/^[^?#]*\//)?.[0];
return importedSpec;
} else {
throw new Error(
`The imported spec "${url}" is not a view spec: ${JSON.stringify(
spec
)}`
);
try {
const result = await fetch(url);
if (!result.ok) {
throw new Error(`${result.status} ${result.statusText}`);
}
importedSpec = await result.json();
} catch (e) {
throw new Error(
`Could not load imported view spec: ${url}. Reason: ${e.message}`
);
}

if (viewContext.isViewSpec(importedSpec)) {
importedSpec.baseUrl = concatUrl(
getDirectory(importParam.url),
importedSpec.baseUrl
);
return importedSpec;
} else {
throw new Error("Not an url import: " + JSON.stringify(importParam));
throw new Error(
`The imported spec "${url}" is not a view spec: ${JSON.stringify(
spec
)}`
);
}
}

Expand Down

0 comments on commit d08bce8

Please sign in to comment.