Skip to content
Open
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
149 changes: 117 additions & 32 deletions src/access/infra/repositories/AccessRepository.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { ApiConfig, DataverseApiAuthMechanism } from '../../../core/infra/repositories/ApiConfig'
import { WriteError } from '../../../core/domain/repositories/WriteError'
import { ApiConstants } from '../../../core/infra/repositories/ApiConstants'
import { ApiRepository } from '../../../core/infra/repositories/ApiRepository'
import {
buildRequestConfig,
buildRequestUrl
} from '../../../core/infra/repositories/apiConfigBuilders'
import { GuestbookResponseDTO } from '../../domain/dtos/GuestbookResponseDTO'
import { IAccessRepository } from '../../domain/repositories/IAccessRepository'

Expand All @@ -13,14 +20,7 @@
const endpoint = this.buildApiEndpoint(`${this.accessResourceName}/datafile`, undefined, fileId)
const queryParams = format ? { signed: true, format } : { signed: true }

return this.doPost(endpoint, guestbookResponse, queryParams)
.then((response) => {
const signedUrl = response.data.data.signedUrl
return signedUrl
})
.catch((error) => {
throw error
})
return await this.submitGuestbookDownload(endpoint, guestbookResponse, queryParams)
}

public async submitGuestbookForDatafilesDownload(
Expand All @@ -30,21 +30,14 @@
): Promise<string> {
const queryParams = format ? { signed: true, format } : { signed: true }

return this.doPost(
return await this.submitGuestbookDownload(
this.buildApiEndpoint(
this.accessResourceName,
`datafiles/${Array.isArray(fileIds) ? fileIds.join(',') : fileIds}`
),
guestbookResponse,
queryParams
)
.then((response) => {
const signedUrl = response.data.data.signedUrl
return signedUrl
})
.catch((error) => {
throw error
})
}

public async submitGuestbookForDatasetDownload(
Expand All @@ -59,14 +52,7 @@
)
const queryParams = format ? { signed: true, format } : { signed: true }

return this.doPost(endpoint, guestbookResponse, queryParams)
.then((response) => {
const signedUrl = response.data.data.signedUrl
return signedUrl
})
.catch((error) => {
throw error
})
return await this.submitGuestbookDownload(endpoint, guestbookResponse, queryParams)
}

public async submitGuestbookForDatasetVersionDownload(
Expand All @@ -82,13 +68,112 @@
)
const queryParams = format ? { signed: true, format } : { signed: true }

return this.doPost(endpoint, guestbookResponse, queryParams)
.then((response) => {
const signedUrl = response.data.data.signedUrl
return signedUrl
})
.catch((error) => {
throw error
})
return await this.submitGuestbookDownload(endpoint, guestbookResponse, queryParams)
}

private async submitGuestbookDownload(
apiEndpoint: string,
guestbookResponse: GuestbookResponseDTO,
queryParams: object
): Promise<string> {
const requestConfig = buildRequestConfig(
true,
queryParams,
ApiConstants.CONTENT_TYPE_APPLICATION_JSON
)
const response = await fetch(
this.buildUrlWithQueryParams(buildRequestUrl(apiEndpoint), queryParams),
{
method: 'POST',
headers: this.buildFetchHeaders(requestConfig.headers),
credentials: this.getFetchCredentials(requestConfig.withCredentials),
body: JSON.stringify(guestbookResponse)
}
).catch((error) => {
Comment on lines +74 to +92
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AccessRepository reimplements request building (query param serialization, header conversion, body parsing, error message formatting, credentials selection) using fetch, while the rest of the codebase standardizes HTTP behavior via ApiRepository + axios. To reduce drift and keep error/timeout/auth behavior consistent, consider extracting these helpers into ApiRepository (e.g., a shared doPostFetch/doRequestFetch used by this repository) or a core utility module reused across repositories.

Copilot uses AI. Check for mistakes.
throw new WriteError(error instanceof Error ? error.message : String(error))
})

const responseData = await this.parseResponseBody(response)

if (!response.ok) {
throw new WriteError(this.buildFetchErrorMessage(response.status, responseData))
}

return this.getSignedUrlOrThrow(responseData)
}
Comment on lines +96 to +103
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After parseResponseBody, the code assumes responseData is an object with data.signedUrl. If the response has a missing/incorrect content-type header (so it’s parsed as text) or the JSON shape differs, this will throw a runtime TypeError instead of a controlled WriteError. Add validation for the expected response shape (and/or a fallback JSON parse attempt) and throw a WriteError with a clear message when signedUrl is absent.

Copilot uses AI. Check for mistakes.

private getFetchCredentials(withCredentials?: boolean): RequestCredentials | undefined {
if (ApiConfig.dataverseApiAuthMechanism === DataverseApiAuthMechanism.BEARER_TOKEN) {
return 'omit'
}

if (withCredentials) {
return 'include'
}

return undefined
}

private buildUrlWithQueryParams(requestUrl: string, queryParams: object): string {
const url = new URL(requestUrl)

Object.entries(queryParams).forEach(([key, value]) => {
if (value !== undefined && value !== null) {
url.searchParams.append(key, String(value))
}
})

return url.toString()
}

private buildFetchHeaders(headers?: Record<string, unknown>): Record<string, string> {
const fetchHeaders: Record<string, string> = {}

if (!headers) {
return fetchHeaders
}

Object.entries(headers).forEach(([key, value]) => {
if (value !== undefined) {
fetchHeaders[key] = String(value)
}
})

return fetchHeaders
}

private async parseResponseBody(response: Response): Promise<any> {

Check warning on line 145 in src/access/infra/repositories/AccessRepository.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
const contentType = response.headers.get('content-type') ?? ''

if (contentType.includes('application/json')) {
return await response.json()
}

const responseText = await response.text()

try {
return JSON.parse(responseText)
} catch {
return responseText
}
}

private buildFetchErrorMessage(status: number, responseData: any): string {

Check warning on line 161 in src/access/infra/repositories/AccessRepository.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
const message =
typeof responseData === 'string'
? responseData
: responseData?.message || responseData?.data?.message || 'unknown error'

return `[${status}] ${message}`
}

private getSignedUrlOrThrow(responseData: any): string {

Check warning on line 170 in src/access/infra/repositories/AccessRepository.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
const signedUrl = responseData?.data?.signedUrl

if (typeof signedUrl !== 'string' || signedUrl.length === 0) {
throw new WriteError('Missing signedUrl in access download response.')
}

return signedUrl
}
}
5 changes: 4 additions & 1 deletion test/integration/access/AccessRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,14 @@ describe('AccessRepository', () => {
})

describe('submitGuestbookForDatafileDownload', () => {
test('should return signed url for datafile download', async () => {
test('should return signed url that can be used to download the datafile', async () => {
const actual = await sut.submitGuestbookForDatafileDownload(testFileId, guestbookResponse)
const downloadResponse = await fetch(actual)

expect(actual).toEqual(expect.any(String))
expect(() => new URL(actual)).not.toThrow()
expect(downloadResponse.ok).toBe(true)
await expect(downloadResponse.text()).resolves.toBe('test file 1\n')
})

test('should return error when datafile does not exist', async () => {
Expand Down
120 changes: 120 additions & 0 deletions test/unit/access/AccessRepository.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/**
* @jest-environment jsdom
*/

import { AccessRepository } from '../../../src/access/infra/repositories/AccessRepository'
import { GuestbookResponseDTO } from '../../../src/access/domain/dtos/GuestbookResponseDTO'
import { WriteError } from '../../../src/core/domain/repositories/WriteError'
import {
ApiConfig,
DataverseApiAuthMechanism
} from '../../../src/core/infra/repositories/ApiConfig'
import { TestConstants } from '../../testHelpers/TestConstants'

describe('AccessRepository', () => {
const sut = new AccessRepository()
const originalFetch = global.fetch
const guestbookResponse: GuestbookResponseDTO = {
guestbookResponse: {
answers: [{ id: 1, value: 'question 1' }]
}
}

beforeEach(() => {
window.localStorage.setItem(
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY,
JSON.stringify(TestConstants.TEST_DUMMY_BEARER_TOKEN)
)
})

afterEach(() => {
global.fetch = originalFetch
window.localStorage.clear()
})

test('uses fetch with credentials omit for bearer token auth', async () => {
ApiConfig.init(
TestConstants.TEST_API_URL,
DataverseApiAuthMechanism.BEARER_TOKEN,
undefined,
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY
)

const fetchMock = jest.fn().mockResolvedValue({
ok: true,
status: 200,
headers: new Headers({ 'content-type': 'application/json' }),
json: jest.fn().mockResolvedValue({
data: {
signedUrl: 'https://signed.dataset'
}
})
} as unknown as Response)

global.fetch = fetchMock as typeof fetch

Comment on lines +43 to +55
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test overwrites global.fetch but never restores it. That can leak into other unit tests running in the same worker and make failures order-dependent. Capture the original global.fetch and restore it in afterEach (or use jest.spyOn(global, 'fetch') and mockRestore()).

Copilot uses AI. Check for mistakes.
const actual = await sut.submitGuestbookForDatasetDownload(123, guestbookResponse, 'original')

expect(fetchMock).toHaveBeenCalledWith(
`${TestConstants.TEST_API_URL}/access/dataset/123?signed=true&format=original`,
{
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${TestConstants.TEST_DUMMY_BEARER_TOKEN}`
},
credentials: 'omit',
body: JSON.stringify(guestbookResponse)
}
)
expect(actual).toBe('https://signed.dataset')
})

test('parses signed url from a JSON body when content-type is incorrect', async () => {
ApiConfig.init(
TestConstants.TEST_API_URL,
DataverseApiAuthMechanism.BEARER_TOKEN,
undefined,
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY
)

const fetchMock = jest.fn().mockResolvedValue({
ok: true,
status: 200,
headers: new Headers({ 'content-type': 'text/plain' }),
text: jest
.fn()
.mockResolvedValue(JSON.stringify({ data: { signedUrl: 'https://signed.text' } }))
} as unknown as Response)

global.fetch = fetchMock as typeof fetch

await expect(sut.submitGuestbookForDatasetDownload(123, guestbookResponse)).resolves.toBe(
'https://signed.text'
)
})

test('throws WriteError when signedUrl is missing from a successful response', async () => {
ApiConfig.init(
TestConstants.TEST_API_URL,
DataverseApiAuthMechanism.BEARER_TOKEN,
undefined,
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY
)

const fetchMock = jest.fn().mockResolvedValue({
ok: true,
status: 200,
headers: new Headers({ 'content-type': 'application/json' }),
json: jest.fn().mockResolvedValue({
data: {}
})
} as unknown as Response)

global.fetch = fetchMock as typeof fetch

await expect(sut.submitGuestbookForDatasetDownload(123, guestbookResponse)).rejects.toThrow(
new WriteError('Missing signedUrl in access download response.')
)
})
})
Loading