Skip to content

Commit 8326ecb

Browse files
fix(web): prevent writing large auspice json to local storage
After `dataset-json-url` param is used, the entire auspice json was stored in local storage. For large files this would be bigger that what browsers allow and the store operation would fail. Subsequent reads would retrieve either nothing or some fragment of data. Here I add some logic to make sure we store auspice json in memory only. Sadly, this means that page reload wipes all the data and we currently don't have a good mechanism to store the previous dataset URL in order to re-fetch it. This will need to be implemented. Currently I opted for discarding the stored dataset if it was auspice dataset. This should be alright, as the feature was primarily designed to be used when navigating to Nextclade with url params set. This PR only implements a workaround for something that's fundamentally poorly designed and broken. The full dataset description object is still saved to local storage. The definitive fix requires serious restructuring and altering many components. It will be implemented later (#1464) But this workaround should allow most use-cases of `dataset-json-url` to work and makes this auspice dataset feature releasable. Can be tested with a large tree like this: ``` ?dataset-json-url=https://nextstrain.org/ncov/gisaid/global/all-time ```
1 parent 29c49e5 commit 8326ecb

File tree

6 files changed

+46
-20
lines changed

6 files changed

+46
-20
lines changed

packages/nextclade-web/.eslintrc.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ module.exports = {
130130
'no-shadow': 'off',
131131
'no-unused-vars': 'off',
132132
'only-ascii/only-ascii': 'warn',
133+
'prefer-const': ['warn', { destructuring: 'all' }],
133134
'prefer-for-of': 'off',
134135
'prettier/prettier': 'warn',
135136
'react-hooks/exhaustive-deps': [
@@ -148,7 +149,7 @@ module.exports = {
148149
'react/state-in-constructor': 'off',
149150
'security/detect-non-literal-fs-filename': 'off',
150151
'security/detect-object-injection': 'off',
151-
'sonarjs/cognitive-complexity': ['warn', 20],
152+
'sonarjs/cognitive-complexity': 'off',
152153
'unicorn/consistent-function-scoping': 'off',
153154
'unicorn/escape-case': 'off',
154155
'unicorn/filename-case': 'off',

packages/nextclade-web/src/io/fetchDatasets.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,7 @@ export async function initializeDatasets(datasetServerUrl: string, urlQuery: Par
128128
const minimizerIndexVersion = await getCompatibleMinimizerIndexVersion(datasetServerUrl, datasetsIndexJson)
129129

130130
// Check if URL params specify dataset params and try to find the corresponding dataset
131-
const currentDataset:
132-
| (Dataset & {
133-
auspiceJson?: AuspiceTree
134-
})
135-
| undefined = await getDatasetFromUrlParams(urlQuery, datasets)
131+
const currentDataset = await getDatasetFromUrlParams(urlQuery, datasets)
136132

137133
return { datasets, currentDataset, minimizerIndexVersion }
138134
}

packages/nextclade-web/src/io/fetchSingleDatasetAuspice.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export async function fetchSingleDatasetAuspice(datasetJsonUrl_: string) {
3939
}
4040
}
4141

42-
const currentDataset: Dataset & { auspiceJson?: AuspiceTree } = {
42+
const currentDataset: Dataset = {
4343
path: datasetJsonUrl,
4444
capabilities: {
4545
primers: false,
@@ -50,8 +50,8 @@ export async function fetchSingleDatasetAuspice(datasetJsonUrl_: string) {
5050
name,
5151
...pathogen?.attributes,
5252
},
53+
type: 'auspiceJson',
5354
version,
54-
auspiceJson,
5555
}
5656

5757
const datasets = [currentDataset]
@@ -60,5 +60,5 @@ export async function fetchSingleDatasetAuspice(datasetJsonUrl_: string) {
6060
const defaultDatasetName = currentDatasetName
6161
const defaultDatasetNameFriendly = attrStrMaybe(currentDataset.attributes, 'name') ?? currentDatasetName
6262

63-
return { datasets, defaultDataset, defaultDatasetName, defaultDatasetNameFriendly, currentDataset }
63+
return { datasets, defaultDataset, defaultDatasetName, defaultDatasetNameFriendly, currentDataset, auspiceJson }
6464
}

packages/nextclade-web/src/io/fetchSingleDatasetDirectory.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import axios from 'axios'
22
import urljoin from 'url-join'
33
import { mapValues } from 'lodash'
44
import { concurrent } from 'fasy'
5-
import { attrStrMaybe, AuspiceTree, Dataset, VirusProperties } from 'src/types'
5+
import { attrStrMaybe, Dataset, VirusProperties } from 'src/types'
66
import { removeTrailingSlash } from 'src/io/url'
77
import { axiosFetch, axiosHead, axiosHeadOrUndefined } from 'src/io/axiosFetch'
88
import { sanitizeError } from 'src/helpers/sanitizeError'
@@ -17,14 +17,15 @@ export async function fetchSingleDatasetDirectory(
1717

1818
const files = mapValues(pathogen.files, (file) => (file ? urljoin(datasetRootUrl, file) : file))
1919

20-
const currentDataset: Dataset & { auspiceJson?: AuspiceTree } = {
20+
const currentDataset: Dataset = {
2121
path: datasetRootUrl,
2222
capabilities: {
2323
primers: false,
2424
qc: [],
2525
},
2626
...pathogen,
2727
files,
28+
type: 'directory',
2829
}
2930

3031
const datasets = [currentDataset]
@@ -51,7 +52,14 @@ export async function fetchSingleDatasetDirectory(
5152
Object.entries(files).filter(([_, key]) => !['examples', 'readme'].includes(key)),
5253
)
5354

54-
return { datasets, defaultDataset, defaultDatasetName, defaultDatasetNameFriendly, currentDataset }
55+
return {
56+
datasets,
57+
defaultDataset,
58+
defaultDatasetName,
59+
defaultDatasetNameFriendly,
60+
currentDataset,
61+
auspiceJson: undefined,
62+
}
5563
}
5664

5765
async function fetchPathogenJson(datasetRootUrl: string) {

packages/nextclade-web/src/pages/_app.tsx

+18-8
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { RecoilEnv, RecoilRoot, useRecoilCallback, useRecoilState, useRecoilValu
88
import { AppProps } from 'next/app'
99
import { useRouter } from 'next/router'
1010
import dynamic from 'next/dynamic'
11-
import type { Dataset, AuspiceTree } from 'src/types'
1211
import { sanitizeError } from 'src/helpers/sanitizeError'
1312
import { useRunAnalysis } from 'src/hooks/useRunAnalysis'
1413
import i18nAuspice, { changeAuspiceLocale } from 'src/i18n/i18n.auspice'
@@ -97,24 +96,35 @@ function RecoilStateInitializer() {
9796

9897
const datasetInfo = await fetchSingleDataset(urlQuery)
9998
if (!isNil(datasetInfo)) {
100-
const { datasets, currentDataset } = datasetInfo
101-
return { datasets, currentDataset, minimizerIndexVersion: undefined }
99+
const { datasets, currentDataset, auspiceJson } = datasetInfo
100+
return { datasets, currentDataset, minimizerIndexVersion: undefined, auspiceJson }
102101
}
103-
return { datasets, currentDataset, minimizerIndexVersion }
102+
return { datasets, currentDataset, minimizerIndexVersion, auspiceJson: undefined }
104103
})
105104
.catch((error) => {
106105
// Dataset error is fatal and we want error to be handled in the ErrorBoundary
107106
setInitialized(false)
108107
set(globalErrorAtom, sanitizeError(error))
109108
throw error
110109
})
111-
.then(async ({ datasets, currentDataset, minimizerIndexVersion }) => {
110+
.then(async ({ datasets, currentDataset, minimizerIndexVersion, auspiceJson }) => {
112111
set(datasetsAtom, { datasets })
113-
const previousDataset = await getPromise(datasetCurrentAtom)
114-
const dataset: (Dataset & { auspiceJson?: AuspiceTree }) | undefined = currentDataset ?? previousDataset
112+
let previousDataset = await getPromise(datasetCurrentAtom)
113+
if (previousDataset?.type === 'auspiceJson') {
114+
// Disregard previously saved dataset if it's Auspice dataset, because the data is no longer available.
115+
// We might re-fetch instead, but need to persist URL for that somehow.
116+
previousDataset = undefined
117+
}
118+
119+
const dataset = currentDataset ?? previousDataset
115120
set(datasetCurrentAtom, dataset)
116121
set(minimizerIndexVersionAtom, minimizerIndexVersion)
117-
set(datasetJsonAtom, dataset?.auspiceJson)
122+
123+
if (dataset?.type === 'auspiceJson' && !isNil(auspiceJson)) {
124+
set(datasetJsonAtom, auspiceJson)
125+
} else {
126+
set(datasetJsonAtom, undefined)
127+
}
118128
return dataset
119129
})
120130
.then(async (dataset) => {

packages/nextclade/src/io/dataset.rs

+11
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ pub struct Dataset {
7878
#[serde(default, skip_serializing_if = "DatasetVersion::is_empty")]
7979
pub version: DatasetVersion,
8080

81+
#[serde(default, skip_serializing_if = "Option::is_none")]
82+
pub r#type: Option<DatasetType>,
83+
8184
#[serde(flatten)]
8285
pub other: serde_json::Value,
8386
}
@@ -386,3 +389,11 @@ pub struct MinimizerIndexVersion {
386389
#[serde(flatten)]
387390
pub other: serde_json::Value,
388391
}
392+
393+
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
394+
#[serde(rename_all = "camelCase")]
395+
pub enum DatasetType {
396+
Directory,
397+
AuspiceJson,
398+
Other,
399+
}

0 commit comments

Comments
 (0)