Skip to content

Commit 6b1ffd8

Browse files
thinknoacknellh
authored andcommitted
update parsedcontrubutors to return empty array and adding spec
1 parent 980d184 commit 6b1ffd8

File tree

2 files changed

+178
-4
lines changed

2 files changed

+178
-4
lines changed
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest"
2+
import yaml from "js-yaml"
3+
import * as Sentry from "@sentry/node"
4+
import CacheItem from "../../cache/item"
5+
import { fileUrl } from "../files"
6+
import { datasetOrSnapshot } from "../../utils/datasetOrSnapshot"
7+
import { contributors } from "../contributors"
8+
9+
vi.mock("../../libs/authentication/jwt", () => ({
10+
sign: vi.fn(() => "mock_jwt_token"),
11+
verify: vi.fn(() => ({ userId: "mock_user_id" })),
12+
}))
13+
14+
vi.mock("js-yaml", () => ({
15+
default: {
16+
load: vi.fn(),
17+
},
18+
}))
19+
20+
vi.mock("@sentry/node", () => ({
21+
captureMessage: vi.fn(),
22+
captureException: vi.fn(),
23+
}))
24+
25+
vi.mock("../../cache/item")
26+
vi.mock("../files")
27+
vi.mock("../../utils/datasetOrSnapshot")
28+
vi.mock("../libs/redis", () => ({
29+
redis: vi.fn(),
30+
}))
31+
32+
const mockYamlLoad = vi.mocked(yaml.load)
33+
const mockSentryCaptureMessage = vi.mocked(Sentry.captureMessage)
34+
const mockSentryCaptureException = vi.mocked(Sentry.captureException)
35+
const mockFileUrl = vi.mocked(fileUrl)
36+
const mockDatasetOrSnapshot = vi.mocked(datasetOrSnapshot)
37+
38+
const mockFetch = vi.fn()
39+
global.fetch = mockFetch
40+
41+
const mockCacheItemGet = vi.fn()
42+
vi.mocked(CacheItem).mockImplementation((_redis, _type, _key) => {
43+
return {
44+
get: mockCacheItemGet,
45+
} as unknown as CacheItem
46+
})
47+
48+
describe("contributors (core functionality)", () => {
49+
const MOCK_DATASET_ID = "ds000001"
50+
const MOCK_REVISION = "dce4b7b6653bcde9bdb7226a7c2b9499e77f2724"
51+
52+
beforeEach(() => {
53+
vi.clearAllMocks()
54+
55+
mockDatasetOrSnapshot.mockReturnValue({
56+
datasetId: MOCK_DATASET_ID,
57+
revision: MOCK_REVISION,
58+
})
59+
mockFileUrl.mockImplementation(
60+
(datasetId, path, filename, revision) =>
61+
`http://example.com/${datasetId}/${revision}/${filename}`,
62+
)
63+
64+
mockCacheItemGet.mockImplementation((fetcher) => fetcher())
65+
})
66+
67+
it("should return empty array if both datacite file and dataset_description.json fail", async () => {
68+
mockFetch.mockResolvedValueOnce({
69+
status: 500,
70+
headers: new Headers(),
71+
text: () => Promise.resolve("Server Error"),
72+
})
73+
mockCacheItemGet.mockImplementationOnce((fetcher) =>
74+
fetcher().catch(() => null)
75+
)
76+
const result = await contributors({
77+
id: MOCK_DATASET_ID,
78+
revision: MOCK_REVISION,
79+
})
80+
expect(result).toEqual([])
81+
expect(mockSentryCaptureException).toHaveBeenCalledTimes(1)
82+
})
83+
84+
it("should return default empty array if no contributors array in datacite file or dataset_description.json (or wrong resourceTypeGeneral in datacite file)", async () => {
85+
const dataciteYamlContent =
86+
`data:\n attributes:\n types:\n resourceTypeGeneral: Software\n contributors: []`
87+
const parsedDatacite = {
88+
data: {
89+
attributes: {
90+
types: { resourceTypeGeneral: "Software" },
91+
contributors: [],
92+
},
93+
},
94+
}
95+
96+
mockFetch.mockResolvedValueOnce({
97+
status: 200,
98+
headers: new Headers({ "Content-Type": "application/yaml" }),
99+
text: () => Promise.resolve(dataciteYamlContent),
100+
})
101+
mockYamlLoad.mockReturnValueOnce(parsedDatacite)
102+
const result = await contributors({
103+
id: MOCK_DATASET_ID,
104+
revision: MOCK_REVISION,
105+
})
106+
expect(result).toEqual([])
107+
expect(mockSentryCaptureMessage).toHaveBeenCalledWith(
108+
`Datacite file for ${MOCK_DATASET_ID}:${MOCK_REVISION} found but resourceTypeGeneral is 'Software', not 'Dataset'.`,
109+
)
110+
expect(mockSentryCaptureException).not.toHaveBeenCalled()
111+
})
112+
113+
it("should return default empty array if datacite file is Dataset type but provides no contributors", async () => {
114+
const dataciteYamlContent =
115+
`data:\n attributes:\n types:\n resourceTypeGeneral: Dataset\n contributors: []`
116+
const parsedDatacite = {
117+
data: {
118+
attributes: {
119+
types: { resourceTypeGeneral: "Dataset" },
120+
contributors: [],
121+
},
122+
},
123+
}
124+
125+
mockFetch.mockResolvedValueOnce({
126+
status: 200,
127+
headers: new Headers({ "Content-Type": "application/yaml" }),
128+
text: () => Promise.resolve(dataciteYamlContent),
129+
})
130+
mockYamlLoad.mockReturnValueOnce(parsedDatacite)
131+
const result = await contributors({
132+
id: MOCK_DATASET_ID,
133+
revision: MOCK_REVISION,
134+
})
135+
136+
expect(result).toEqual([])
137+
expect(mockSentryCaptureMessage).toHaveBeenCalledWith(
138+
`Datacite file for ${MOCK_DATASET_ID}:${MOCK_REVISION} is Dataset type but provided no contributors.`,
139+
)
140+
expect(mockSentryCaptureException).not.toHaveBeenCalled()
141+
})
142+
143+
it("should capture message if datacite file has unexpected content type but still parses", async () => {
144+
const dataciteYamlContent =
145+
`data:\n attributes:\n types:\n resourceTypeGeneral: Dataset\n contributors: []`
146+
const parsedDatacite = {
147+
data: {
148+
attributes: {
149+
types: { resourceTypeGeneral: "Dataset" },
150+
contributors: [],
151+
},
152+
},
153+
}
154+
155+
mockFetch.mockResolvedValueOnce({
156+
status: 200,
157+
headers: new Headers({ "Content-Type": "text/plain" }),
158+
text: () => Promise.resolve(dataciteYamlContent),
159+
})
160+
mockYamlLoad.mockReturnValueOnce(parsedDatacite)
161+
const result = await contributors({
162+
id: MOCK_DATASET_ID,
163+
revision: MOCK_REVISION,
164+
})
165+
expect(mockSentryCaptureMessage).toHaveBeenCalledWith(
166+
expect.stringContaining(
167+
`Datacite file for ${MOCK_DATASET_ID}:${MOCK_REVISION} served with unexpected Content-Type: text/plain. Attempting YAML parse anyway.`,
168+
),
169+
)
170+
expect(mockSentryCaptureException).not.toHaveBeenCalled()
171+
expect(mockSentryCaptureMessage).toHaveBeenCalledWith(
172+
`Datacite file for ${MOCK_DATASET_ID}:${MOCK_REVISION} is Dataset type but provided no contributors.`,
173+
)
174+
expect(result).toEqual([])
175+
})
176+
})

packages/openneuro-server/src/datalad/contributors.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,7 @@ export const contributors = async (
5757
const { datasetId, revision } = datasetOrSnapshot(obj)
5858
const revisionShort = revision.substring(0, 7)
5959

60-
// Default fallback for authors if neither file provides them
61-
const defaultContributors: Contributor[] = []
62-
let parsedContributors: Contributor[] | null = null
60+
let parsedContributors: Contributor[] = []
6361

6462
// 1. Try to get contributors from datacite
6563
const dataciteCache = new CacheItem(redis, CacheType.dataciteYml, [
@@ -101,5 +99,5 @@ export const contributors = async (
10199
}
102100

103101
// Return the parsed contributors or the default empty array
104-
return parsedContributors || defaultContributors
102+
return parsedContributors
105103
}

0 commit comments

Comments
 (0)