Skip to content

Commit f04c9b1

Browse files
effigieseffigies
committed
fix: Resolve circular dependencies by deferred access and a new module
1 parent 397c1c8 commit f04c9b1

6 files changed

Lines changed: 143 additions & 138 deletions

File tree

‎packages/openneuro-server/src/datalad/snapshots.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@ import request from "superagent"
66
import { getRedis, getRedlock } from "../libs/redis"
77
import CacheItem, { CacheType } from "../cache/item"
88
import config from "../config"
9-
import {
10-
snapshotCreationComparison,
11-
updateDatasetName,
12-
} from "../graphql/resolvers/dataset"
9+
import { snapshotCreationComparison } from "../utils/snapshots"
1310
import { createDraftDoi } from "../libs/doi/index"
1411
import { assembleMetadata } from "../libs/doi/metadata"
1512
import Doi from "../models/doi"
@@ -187,7 +184,10 @@ export const createSnapshot = async (
187184
createSnapshotMetadata(datasetId, tag, snapshot.hexsha, snapshot.created),
188185

189186
// Trigger an async update for the name field (cache for sorting)
190-
updateDatasetName(datasetId),
187+
// Dynamic import breaks circular dependency: datalad/snapshots → resolvers/dataset
188+
import("../graphql/resolvers/dataset").then((m) =>
189+
m.updateDatasetName(datasetId)
190+
),
191191
])
192192

193193
const snapshotListCache = new CacheItem(getRedis(), CacheType.snapshot, [

‎packages/openneuro-server/src/graphql/resolvers/__tests__/dataset.spec.ts‎

Lines changed: 0 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -29,124 +29,6 @@ describe("dataset resolvers", () => {
2929
expect(dsId).toEqual(expect.stringMatching(/^ds[0-9]{6}$/))
3030
})
3131
})
32-
describe("snapshotCreationComparison()", () => {
33-
it('sorts array of objects by the "created" and "tag" properties', () => {
34-
const testArray = [
35-
{ id: 2, created: new Date("2018-11-20T00:05:43.473Z"), tag: "1.0.0" },
36-
{ id: 1, created: new Date("2018-11-19T00:05:43.473Z"), tag: "1.0.1" },
37-
{ id: 3, created: new Date("2018-11-23T00:05:43.473Z"), tag: "1.0.2" },
38-
{ id: 5, created: new Date("2018-11-23T00:05:43.473Z"), tag: "1.0.10" },
39-
{ id: 4, created: new Date("2018-11-23T00:05:43.473Z"), tag: "1.0.3" },
40-
]
41-
const sorted = testArray.sort(ds.snapshotCreationComparison)
42-
expect(sorted[0].id).toBe(2)
43-
expect(sorted[1].id).toBe(1)
44-
expect(sorted[2].id).toBe(3)
45-
expect(sorted[3].id).toBe(4)
46-
expect(sorted[4].id).toBe(5)
47-
})
48-
it('sorts array of objects by the "created" property as strings', () => {
49-
const testArray = [
50-
{ id: 2, created: "2018-11-20T00:05:43.473Z", tag: "2.0.0" },
51-
{ id: 1, created: "2018-11-19T00:05:43.473Z", tag: "1.0.0" },
52-
{ id: 3, created: "2018-11-23T00:05:43.473Z", tag: "3.0.0" },
53-
]
54-
const sorted = testArray.sort(ds.snapshotCreationComparison)
55-
expect(sorted[0].id).toBe(1)
56-
expect(sorted[1].id).toBe(2)
57-
expect(sorted[2].id).toBe(3)
58-
})
59-
it("sorts non-semver tags mixed with semver tags", () => {
60-
const testArray = [
61-
{ id: 2, created: new Date("2018-11-19T00:05:43.473Z"), tag: "1.0.2" },
62-
{
63-
id: 1,
64-
created: new Date("2018-11-19T00:05:43.473Z"),
65-
tag: "57fed018cce88d000ac1757f",
66-
},
67-
{ id: 3, created: new Date("2018-11-19T00:05:43.473Z"), tag: "1.0.1" },
68-
]
69-
const sorted = testArray.sort(ds.snapshotCreationComparison)
70-
expect(sorted[0].id).toBe(2)
71-
expect(sorted[1].id).toBe(1)
72-
expect(sorted[2].id).toBe(3)
73-
})
74-
it("sorts snapshots with only non-semver tags", () => {
75-
const testArray = [
76-
{
77-
id: 2,
78-
created: new Date("2018-11-19T00:05:43.473Z"),
79-
tag: "00001",
80-
},
81-
{
82-
id: 1,
83-
created: new Date("2018-11-19T00:05:43.473Z"),
84-
tag: "57fed018cce88d000ac1757f",
85-
},
86-
{
87-
id: 3,
88-
created: new Date("2018-11-19T00:05:43.473Z"),
89-
tag: "57fed018cce88d000ac1757f",
90-
},
91-
]
92-
const sorted = testArray.sort(ds.snapshotCreationComparison)
93-
expect(sorted[0].id).toBe(2)
94-
expect(sorted[1].id).toBe(1)
95-
expect(sorted[2].id).toBe(3)
96-
})
97-
it("sorts very similar creation times by semver order", () => {
98-
const testSnapshots = [
99-
{
100-
id: "ds002680:1.0.0",
101-
created: "2020-04-03T23:19:56.000Z",
102-
tag: "1.0.0",
103-
},
104-
{
105-
id: "ds002680:1.2.0",
106-
created: "2021-10-19T16:26:43.000Z",
107-
tag: "1.2.0",
108-
},
109-
{
110-
id: "ds002680:1.1.0",
111-
created: "2021-10-19T16:26:44.000Z",
112-
tag: "1.1.0",
113-
},
114-
]
115-
const sorted = testSnapshots.sort(ds.snapshotCreationComparison)
116-
expect(sorted[0].id).toBe("ds002680:1.0.0")
117-
expect(sorted[1].id).toBe("ds002680:1.1.0")
118-
expect(sorted[2].id).toBe("ds002680:1.2.0")
119-
})
120-
it("sorts 000002 (legacy snapshots) before 1.0.1 (current format)", () => {
121-
const testSnapshots = [
122-
{
123-
id: "ds000247:00002",
124-
created: "2018-07-18T02:27:39.000Z",
125-
tag: "00002",
126-
},
127-
{
128-
id: "ds000247:00001",
129-
created: "2018-07-18T02:35:37.000Z",
130-
tag: "00001",
131-
},
132-
{
133-
id: "ds000247:1.0.0",
134-
created: "2021-07-05T15:58:18.000Z",
135-
tag: "1.0.0",
136-
},
137-
{
138-
id: "ds000247:1.0.1",
139-
created: "2021-08-25T23:37:53.000Z",
140-
tag: "1.0.1",
141-
},
142-
]
143-
const sorted = testSnapshots.sort(ds.snapshotCreationComparison)
144-
expect(sorted[0].id).toBe("ds000247:00002")
145-
expect(sorted[1].id).toBe("ds000247:00001")
146-
expect(sorted[2].id).toBe("ds000247:1.0.0")
147-
expect(sorted[3].id).toBe("ds000247:1.0.1")
148-
})
149-
})
15032
describe("deleteFiles", () => {
15133
beforeEach(() => {
15234
request.post.mockClear()

‎packages/openneuro-server/src/graphql/resolvers/dataset.ts‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as datalad from "../../datalad/dataset"
22
import { removeDatasetSearchDocument } from "./dataset-search"
3+
import { snapshotCreationComparison } from "../../utils/snapshots"
34
import { latestSnapshot, snapshots } from "./snapshots"
45
import { description } from "./description"
56
import {
@@ -23,7 +24,6 @@ import { brainInitiative } from "./brainInitiative"
2324
import { derivatives } from "./derivatives"
2425
import { promiseTimeout } from "../../utils/promiseTimeout"
2526
import { datasetEvents } from "./datasetEvents"
26-
import semver from "semver"
2727
import { getDraftInfo } from "../../datalad/draft"
2828
import type { GraphQLContext } from "../builder"
2929
import type { DatasetDocument } from "../../models/dataset"
@@ -53,17 +53,6 @@ export const datasets = (
5353
}
5454
}
5555

56-
export const snapshotCreationComparison = (
57-
{ created: a, tag: a_tag }: { created: Date | string; tag: string },
58-
{ created: b, tag: b_tag }: { created: Date | string; tag: string },
59-
) => {
60-
if (semver.valid(a_tag) && semver.valid(b_tag)) {
61-
return semver.compare(a_tag, b_tag)
62-
} else {
63-
return new Date(a).getTime() - new Date(b).getTime()
64-
}
65-
}
66-
6756
/**
6857
* Find the canonical name for a dataset from snapshots and drafts
6958
* @param {object} obj Dataset object (at least {id: "datasetId"})
@@ -330,8 +319,9 @@ const Dataset = {
330319
modified: draftHead.modified,
331320
}
332321
},
333-
snapshots,
334-
latestSnapshot,
322+
// Wrapper functions defer access to break the resolvers/dataset ↔ resolvers/snapshots cycle
323+
snapshots: (obj) => snapshots(obj),
324+
latestSnapshot: (obj, args, context) => latestSnapshot(obj, args, context),
335325
analytics,
336326
stars,
337327
followers,

‎packages/openneuro-server/src/graphql/resolvers/snapshots.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as datalad from "../../datalad/snapshots"
2-
import { analytics, dataset, snapshotCreationComparison } from "./dataset.js"
2+
import { analytics, dataset } from "./dataset.js"
3+
import { snapshotCreationComparison } from "../../utils/snapshots"
34
import { onBrainlife } from "./brainlife"
45
import { checkDatasetRead, checkDatasetWrite } from "../permissions"
56
import { readme } from "./readme.js"

‎packages/openneuro-server/src/utils/__tests__/snapshots.spec.ts‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { snapshotCreationComparison } from "../snapshots"
2+
3+
describe("snapshotCreationComparison()", () => {
4+
it('sorts array of objects by the "created" and "tag" properties', () => {
5+
const testArray = [
6+
{ id: 2, created: new Date("2018-11-20T00:05:43.473Z"), tag: "1.0.0" },
7+
{ id: 1, created: new Date("2018-11-19T00:05:43.473Z"), tag: "1.0.1" },
8+
{ id: 3, created: new Date("2018-11-23T00:05:43.473Z"), tag: "1.0.2" },
9+
{ id: 5, created: new Date("2018-11-23T00:05:43.473Z"), tag: "1.0.10" },
10+
{ id: 4, created: new Date("2018-11-23T00:05:43.473Z"), tag: "1.0.3" },
11+
]
12+
const sorted = testArray.sort(snapshotCreationComparison)
13+
expect(sorted[0].id).toBe(2)
14+
expect(sorted[1].id).toBe(1)
15+
expect(sorted[2].id).toBe(3)
16+
expect(sorted[3].id).toBe(4)
17+
expect(sorted[4].id).toBe(5)
18+
})
19+
it('sorts array of objects by the "created" property as strings', () => {
20+
const testArray = [
21+
{ id: 2, created: "2018-11-20T00:05:43.473Z", tag: "2.0.0" },
22+
{ id: 1, created: "2018-11-19T00:05:43.473Z", tag: "1.0.0" },
23+
{ id: 3, created: "2018-11-23T00:05:43.473Z", tag: "3.0.0" },
24+
]
25+
const sorted = testArray.sort(snapshotCreationComparison)
26+
expect(sorted[0].id).toBe(1)
27+
expect(sorted[1].id).toBe(2)
28+
expect(sorted[2].id).toBe(3)
29+
})
30+
it("sorts non-semver tags mixed with semver tags", () => {
31+
const testArray = [
32+
{ id: 2, created: new Date("2018-11-19T00:05:43.473Z"), tag: "1.0.2" },
33+
{
34+
id: 1,
35+
created: new Date("2018-11-19T00:05:43.473Z"),
36+
tag: "57fed018cce88d000ac1757f",
37+
},
38+
{ id: 3, created: new Date("2018-11-19T00:05:43.473Z"), tag: "1.0.1" },
39+
]
40+
const sorted = testArray.sort(snapshotCreationComparison)
41+
expect(sorted[0].id).toBe(2)
42+
expect(sorted[1].id).toBe(1)
43+
expect(sorted[2].id).toBe(3)
44+
})
45+
it("sorts snapshots with only non-semver tags", () => {
46+
const testArray = [
47+
{
48+
id: 2,
49+
created: new Date("2018-11-19T00:05:43.473Z"),
50+
tag: "00001",
51+
},
52+
{
53+
id: 1,
54+
created: new Date("2018-11-19T00:05:43.473Z"),
55+
tag: "57fed018cce88d000ac1757f",
56+
},
57+
{
58+
id: 3,
59+
created: new Date("2018-11-19T00:05:43.473Z"),
60+
tag: "57fed018cce88d000ac1757f",
61+
},
62+
]
63+
const sorted = testArray.sort(snapshotCreationComparison)
64+
expect(sorted[0].id).toBe(2)
65+
expect(sorted[1].id).toBe(1)
66+
expect(sorted[2].id).toBe(3)
67+
})
68+
it("sorts very similar creation times by semver order", () => {
69+
const testSnapshots = [
70+
{
71+
id: "ds002680:1.0.0",
72+
created: "2020-04-03T23:19:56.000Z",
73+
tag: "1.0.0",
74+
},
75+
{
76+
id: "ds002680:1.2.0",
77+
created: "2021-10-19T16:26:43.000Z",
78+
tag: "1.2.0",
79+
},
80+
{
81+
id: "ds002680:1.1.0",
82+
created: "2021-10-19T16:26:44.000Z",
83+
tag: "1.1.0",
84+
},
85+
]
86+
const sorted = testSnapshots.sort(snapshotCreationComparison)
87+
expect(sorted[0].id).toBe("ds002680:1.0.0")
88+
expect(sorted[1].id).toBe("ds002680:1.1.0")
89+
expect(sorted[2].id).toBe("ds002680:1.2.0")
90+
})
91+
it("sorts 000002 (legacy snapshots) before 1.0.1 (current format)", () => {
92+
const testSnapshots = [
93+
{
94+
id: "ds000247:00002",
95+
created: "2018-07-18T02:27:39.000Z",
96+
tag: "00002",
97+
},
98+
{
99+
id: "ds000247:00001",
100+
created: "2018-07-18T02:35:37.000Z",
101+
tag: "00001",
102+
},
103+
{
104+
id: "ds000247:1.0.0",
105+
created: "2021-07-05T15:58:18.000Z",
106+
tag: "1.0.0",
107+
},
108+
{
109+
id: "ds000247:1.0.1",
110+
created: "2021-08-25T23:37:53.000Z",
111+
tag: "1.0.1",
112+
},
113+
]
114+
const sorted = testSnapshots.sort(snapshotCreationComparison)
115+
expect(sorted[0].id).toBe("ds000247:00002")
116+
expect(sorted[1].id).toBe("ds000247:00001")
117+
expect(sorted[2].id).toBe("ds000247:1.0.0")
118+
expect(sorted[3].id).toBe("ds000247:1.0.1")
119+
})
120+
})

‎packages/openneuro-server/src/utils/snapshots.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import semver from "semver"
2+
3+
export const snapshotCreationComparison = (
4+
{ created: a, tag: a_tag }: { created: Date | string; tag: string },
5+
{ created: b, tag: b_tag }: { created: Date | string; tag: string },
6+
) => {
7+
if (semver.valid(a_tag) && semver.valid(b_tag)) {
8+
return semver.compare(a_tag, b_tag)
9+
} else {
10+
return new Date(a).getTime() - new Date(b).getTime()
11+
}
12+
}

0 commit comments

Comments
 (0)