diff --git a/CHANGELOG.md b/CHANGELOG.md index 650ee39b..906db1ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ This changelog follows the principles of [Keep a Changelog](https://keepachangel ### Changed +- Files: Added an optional dataset `version` parameter to `getFileCitationByFormat` for version-specific file citation exports. + ### Fixed ### Removed diff --git a/docs/useCases.md b/docs/useCases.md index 79b7e853..6fd3f04f 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -2016,9 +2016,11 @@ import { FileCitationFormat, getFileCitationByFormat } from '@iqss/dataverse-cli const fileId = 3 -getFileCitationByFormat.execute(fileId, FileCitationFormat.BIBTEX).then((citationText: string) => { - /* ... */ -}) +getFileCitationByFormat + .execute(fileId, FileCitationFormat.BIBTEX, '1.0') + .then((citationText: string) => { + /* ... */ + }) /* ... */ ``` @@ -2027,6 +2029,8 @@ _See [use case](../src/files/domain/useCases/GetFileCitationByFormat.ts) impleme The `fileId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers. +The optional third parameter, `version`, selects the **dataset version** whose file metadata is used in the citation. It accepts a numbered version such as `1.0`, or `DatasetNotNumberedVersion.DRAFT`, `LATEST`, or `LATEST_PUBLISHED` (`:draft`, `:latest`, `:latest-published`). When omitted, no version query parameter is sent, it will return the `:latest`. + The `format` parameter must be one of the available [FileCitationFormat](../src/files/domain/models/FileCitationFormat.ts) enum values: `FileCitationFormat.ENDNOTE`, `FileCitationFormat.RIS`, `FileCitationFormat.BIBTEX`, `FileCitationFormat.CSL`, or `FileCitationFormat.INTERNAL`. #### Get File Counts in a Dataset diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index 29f78d6b..9656f5e0 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -58,7 +58,11 @@ export interface IFilesRepository { includeDeaccessioned: boolean ): Promise - getFileCitationByFormat(fileId: number | string, format: FileCitationFormat): Promise + getFileCitationByFormat( + fileId: number | string, + format: FileCitationFormat, + version?: string + ): Promise getFileUploadDestination(datasetId: number | string, file: File): Promise diff --git a/src/files/domain/useCases/GetFileCitationByFormat.ts b/src/files/domain/useCases/GetFileCitationByFormat.ts index 04dc885e..51bd749d 100644 --- a/src/files/domain/useCases/GetFileCitationByFormat.ts +++ b/src/files/domain/useCases/GetFileCitationByFormat.ts @@ -14,9 +14,14 @@ export class GetFileCitationByFormat implements UseCase { * * @param {number | string} [fileId] - The File identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers). * @param {FileCitationFormat} [format] - The citation format to return. + * @param {string} [version] - Dataset version: a number such as 1.0, :draft, :latest, or :latest-published. Omit to use the server default. Drafts require access to the dataset. * @returns {Promise} */ - async execute(fileId: number | string, format: FileCitationFormat): Promise { - return await this.filesRepository.getFileCitationByFormat(fileId, format) + async execute( + fileId: number | string, + format: FileCitationFormat, + version?: string + ): Promise { + return await this.filesRepository.getFileCitationByFormat(fileId, format, version) } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index fbff81e6..2276428f 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -237,12 +237,14 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { public async getFileCitationByFormat( fileId: number | string, - format: FileCitationFormat + format: FileCitationFormat, + version?: string ): Promise { - return this.doGet( - this.buildApiEndpoint(this.accessResourceName, `citation/${format}`, fileId), - true - ) + const endpoint = this.buildApiEndpoint(this.accessResourceName, `citation/${format}`, fileId) + const request = + version === undefined ? this.doGet(endpoint, true) : this.doGet(endpoint, true, { version }) + + return request .then((response) => typeof response.data === 'string' ? response.data : JSON.stringify(response.data) ) diff --git a/test/functional/files/GetFileCitationByFormat.test.ts b/test/functional/files/GetFileCitationByFormat.test.ts index 643a0faf..39a67032 100644 --- a/test/functional/files/GetFileCitationByFormat.test.ts +++ b/test/functional/files/GetFileCitationByFormat.test.ts @@ -5,14 +5,21 @@ import { FileCitationFormat, getDatasetFiles, getFileCitationByFormat, - ReadError + publishDataset, + updateFileMetadata, + VersionUpdateType } from '../../../src' import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig' import { createCollectionViaApi, + publishCollectionViaApi, deleteCollectionViaApi } from '../../testHelpers/collections/collectionHelper' -import { deleteUnpublishedDatasetViaApi } from '../../testHelpers/datasets/datasetHelper' +import { + deletePublishedDatasetViaApi, + waitForNoLocks, + deleteUnpublishedDatasetViaApi +} from '../../testHelpers/datasets/datasetHelper' import { uploadFileViaApi } from '../../testHelpers/files/filesHelper' import { TestConstants } from '../../testHelpers/TestConstants' @@ -113,11 +120,50 @@ describe('execute', () => { expect(citation).toMatch(/ { + const dataset = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + testCollectionAlias + ) + try { + await uploadFileViaApi(dataset.numericId, testTextFile1Name) + const fileId = (await getDatasetFiles.execute(dataset.numericId)).files[0].id + await publishCollectionViaApi(testCollectionAlias) + await publishDataset.execute(dataset.numericId, VersionUpdateType.MAJOR) + await waitForNoLocks(dataset.numericId) + await updateFileMetadata.execute(fileId, { label: 'renamed-file.txt' }) + expect( + await getFileCitationByFormat.execute(fileId, FileCitationFormat.ENDNOTE, ':draft') + ).toContain('renamed-file.txt') + await publishDataset.execute(dataset.numericId, VersionUpdateType.MINOR) + await waitForNoLocks(dataset.numericId) + expect( + await getFileCitationByFormat.execute(fileId, FileCitationFormat.ENDNOTE, '1.1') + ).toContain('renamed-file.txt') + expect( + await getFileCitationByFormat.execute(fileId, FileCitationFormat.ENDNOTE, '1.0') + ).toContain(`${testTextFile1Name}`) + expect( + await getFileCitationByFormat.execute(fileId, FileCitationFormat.RIS, '1.0') + ).toContain(`C1 - ${testTextFile1Name}`) + await uploadFileViaApi(dataset.numericId, 'test-file-2.txt') + const newFile = (await getDatasetFiles.execute(dataset.numericId)).files.find( + (file) => file.id !== fileId + ) + if (!newFile) throw new Error('Newly uploaded file was not returned') + await expect( + getFileCitationByFormat.execute(newFile.id, FileCitationFormat.ENDNOTE, '1.0') + ).rejects.toThrow('[400] File not found in dataset version: 1.0') + } finally { + await deletePublishedDatasetViaApi(dataset.persistentId) + } + }) + test('should throw an error when the file id does not exist', async () => { - const nonExistentFileId = 5 + const nonExistentFileId = 2147483647 await expect( getFileCitationByFormat.execute(nonExistentFileId, FileCitationFormat.BIBTEX) - ).rejects.toThrow(ReadError) + ).rejects.toThrow(/\[404\]/) }) }) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 889a0846..0231f6d6 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -30,7 +30,18 @@ import { import { FileModel } from '../../../src/files/domain/models/FileModel' import { FileCounts } from '../../../src/files/domain/models/FileCounts' import { FileCitationFormat } from '../../../src/files/domain/models/FileCitationFormat' -import { FileDownloadSizeMode, WriteError } from '../../../src' +import { + FileDownloadSizeMode, + WriteError, + VersionUpdateType, + deaccessionDataset, + restrictFile, + getDatasetFiles, + getMaxEmbargoDurationInMonths, + publishDataset, + updateFileMetadata +} from '../../../src' +import { createBuiltInUser } from '../../testHelpers/users/builtinUserApiHelper' import { deaccessionDatasetViaApi, publishDatasetViaApi, @@ -658,6 +669,197 @@ describe('FilesRepository', () => { }) describe('getFileCitationByFormat', () => { + describe('version-specific file citations', () => { + const alias = `citationVersion${Date.now()}` + const originalLabel = 'test-file-1.txt' + const draftLabel = 'renamed-citation-file.txt' + let dataset: CreatedDatasetIdentifiers + let fileId: number + let persistentId: string + let otherUserKey: string + + const authenticate = (key?: string) => { + ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.API_KEY, key) + } + + beforeAll(async () => { + authenticate(process.env.TEST_API_KEY) + await createCollectionViaApi(alias) + await publishCollectionViaApi(alias) + dataset = await createDataset.execute(TestConstants.TEST_NEW_DATASET_DTO, alias) + await uploadFileViaApi(dataset.numericId, originalLabel) + const files = await getDatasetFiles.execute(dataset.numericId) + fileId = files.files[0].id + await registerFileViaApi(fileId) + persistentId = (await getDatasetFiles.execute(dataset.numericId)).files[0].persistentId + await publishDataset.execute(dataset.numericId, VersionUpdateType.MAJOR) + await waitForNoLocks(dataset.numericId) + await updateFileMetadata.execute(fileId, { label: draftLabel }) + otherUserKey = await createBuiltInUser(`citationReader${Date.now()}`) + }) + + beforeEach(() => authenticate(process.env.TEST_API_KEY)) + afterEach(() => authenticate(process.env.TEST_API_KEY)) + + afterAll(async () => { + authenticate(process.env.TEST_API_KEY) + if (dataset) await deletePublishedDatasetViaApi(dataset.persistentId) + await deleteCollectionViaApi(alias) + }) + + test('selects archived metadata and rejects a deaccessioned version', async () => { + const lifecycleDataset = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + alias + ) + try { + await uploadFileViaApi(lifecycleDataset.numericId, originalLabel) + const lifecycleFileId = (await getDatasetFiles.execute(lifecycleDataset.numericId)) + .files[0].id + await publishDataset.execute(lifecycleDataset.numericId, VersionUpdateType.MAJOR) + await waitForNoLocks(lifecycleDataset.numericId) + await updateFileMetadata.execute(lifecycleFileId, { label: draftLabel }) + await publishDataset.execute(lifecycleDataset.numericId, VersionUpdateType.MINOR) + await waitForNoLocks(lifecycleDataset.numericId) + expect( + await sut.getFileCitationByFormat(lifecycleFileId, FileCitationFormat.ENDNOTE, '1.0') + ).toContain(`${originalLabel}`) + await deaccessionDataset.execute(lifecycleDataset.numericId, '1.0', { + deaccessionReason: 'Test version-specific citations' + }) + await expect( + sut.getFileCitationByFormat(lifecycleFileId, FileCitationFormat.ENDNOTE, '1.0') + ).rejects.toThrow('[400] Dataset version not found: 1.0') + } finally { + await deletePublishedDatasetViaApi(lifecycleDataset.persistentId) + } + }) + + test.each([ + { state: 'restricted', restricted: true, embargoed: false }, + { state: 'embargoed', restricted: false, embargoed: true }, + { state: 'embargoed and restricted', restricted: true, embargoed: true } + ])('requires file access for $state file citations', async ({ restricted, embargoed }) => { + const embargoSetting = '/admin/settings/:MaxEmbargoDurationInMonths' + let originalEmbargoDuration: number | undefined + let embargoSettingChanged = false + const restrictedDataset = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + alias + ) + try { + await uploadFileViaApi(restrictedDataset.numericId, originalLabel) + const restrictedFileId = (await getDatasetFiles.execute(restrictedDataset.numericId)) + .files[0].id + if (restricted) { + await restrictFile.execute(restrictedFileId, { + restrict: true, + enableAccessRequest: true + }) + } + if (embargoed) { + try { + originalEmbargoDuration = await getMaxEmbargoDurationInMonths.execute() + } catch (error) { + if (!(error instanceof ReadError) || !error.message.includes('[404]')) throw error + } + await sut.doPut(embargoSetting, '1') + embargoSettingChanged = true + const dateAvailable = new Date() + dateAvailable.setUTCDate(dateAvailable.getUTCDate() + 7) + // No public SDK use case currently sets file embargoes. + await sut.doPost( + `/datasets/${restrictedDataset.numericId}/files/actions/:set-embargo`, + { + fileIds: [restrictedFileId], + dateAvailable: dateAvailable.toISOString().slice(0, 10), + reason: 'Test version-specific citations' + } + ) + } + await publishDataset.execute(restrictedDataset.numericId, VersionUpdateType.MAJOR) + await waitForNoLocks(restrictedDataset.numericId) + expect( + await sut.getFileCitationByFormat(restrictedFileId, FileCitationFormat.ENDNOTE, '1.0') + ).toContain(`${originalLabel}`) + authenticate(otherUserKey) + await expect( + sut.getFileCitationByFormat(restrictedFileId, FileCitationFormat.ENDNOTE, '1.0') + ).rejects.toThrow(/\[403\]/) + } finally { + authenticate(process.env.TEST_API_KEY) + try { + await deletePublishedDatasetViaApi(restrictedDataset.persistentId) + } finally { + if (embargoSettingChanged) { + if (originalEmbargoDuration === undefined) { + await sut.doDelete(embargoSetting) + } else { + await sut.doPut(embargoSetting, String(originalEmbargoDuration)) + } + } + } + } + }) + + describe.each(['numeric', 'persistent'])('%s identifier', (identifierType) => { + let identifier: number | string + beforeEach(() => { + identifier = identifierType === 'numeric' ? fileId : persistentId + }) + + test.each([ + ['1.0', originalLabel], + [DatasetNotNumberedVersion.LATEST_PUBLISHED, originalLabel], + [DatasetNotNumberedVersion.DRAFT, draftLabel], + [DatasetNotNumberedVersion.LATEST, draftLabel] + ])('selects metadata for %s with authorized credentials', async (version, label) => { + const citation = await sut.getFileCitationByFormat( + identifier, + FileCitationFormat.ENDNOTE, + version + ) + expect(citation).toContain(`${label}`) + }) + + test.each([ + '1.0', + DatasetNotNumberedVersion.LATEST_PUBLISHED, + DatasetNotNumberedVersion.LATEST + ])('returns published metadata anonymously for %s', async (version) => { + authenticate() + const citation = await sut.getFileCitationByFormat( + identifier, + FileCitationFormat.ENDNOTE, + version + ) + expect(citation).toContain(`${originalLabel}`) + }) + + test('rejects anonymous access to a draft', async () => { + authenticate() + await expect( + sut.getFileCitationByFormat(identifier, FileCitationFormat.ENDNOTE, ':draft') + ).rejects.toThrow(/\[401\]/) + }) + + test('rejects a real user without draft permissions', async () => { + authenticate(otherUserKey) + await expect( + sut.getFileCitationByFormat(identifier, FileCitationFormat.ENDNOTE, ':draft') + ).rejects.toThrow( + /\[401\] User @citationReader\d+ is not permitted to perform requested action/ + ) + }) + + test.each(['666.0', 'invalid'])('rejects invalid dataset version %s', async (version) => { + await expect( + sut.getFileCitationByFormat(identifier, FileCitationFormat.ENDNOTE, version) + ).rejects.toThrow(/\[400\]/) + }) + }) + }) + test('should return EndNote citation as XML', async () => { const citation = await sut.getFileCitationByFormat(testFileId, FileCitationFormat.ENDNOTE) diff --git a/test/unit/files/FilesRepository.test.ts b/test/unit/files/FilesRepository.test.ts index aa33e7e6..f1729abf 100644 --- a/test/unit/files/FilesRepository.test.ts +++ b/test/unit/files/FilesRepository.test.ts @@ -1166,6 +1166,30 @@ describe('FilesRepository', () => { }) describe('getFileCitationByFormat', () => { + describe.each([123, 'doi:10.5072/FK2/TEST/FILE'])('file identifier %s', (fileId) => { + test.each([undefined, '1.0', ':draft', ':latest', ':latest-published'])( + 'should send version %s with authentication', + async (version) => { + jest.spyOn(axios, 'get').mockResolvedValue({ data: 'citation' }) + const identifier = + typeof fileId === 'number' + ? `${fileId}/citation/EndNote` + : `:persistentId/citation/EndNote?persistentId=${fileId}` + + await expect( + sut.getFileCitationByFormat(fileId, FileCitationFormat.ENDNOTE, version) + ).resolves.toBe('citation') + expect(axios.get).toHaveBeenCalledWith( + `${TestConstants.TEST_API_URL}/access/datafile/${identifier}`, + { + ...TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY, + params: version === undefined ? {} : { version } + } + ) + } + ) + }) + test.each([ { format: FileCitationFormat.ENDNOTE, diff --git a/test/unit/files/GetFileCitationByFormat.test.ts b/test/unit/files/GetFileCitationByFormat.test.ts index d5bd8772..23a3c7fc 100644 --- a/test/unit/files/GetFileCitationByFormat.test.ts +++ b/test/unit/files/GetFileCitationByFormat.test.ts @@ -43,7 +43,30 @@ describe('execute', () => { const actual = await sut.execute(testId, format) expect(actual).toEqual(citation) - expect(filesRepositoryStub.getFileCitationByFormat).toHaveBeenCalledWith(testId, format) + expect(filesRepositoryStub.getFileCitationByFormat).toHaveBeenCalledWith( + testId, + format, + undefined + ) + } + ) + + test.each(['1.0', ':draft', ':latest', ':latest-published'])( + 'should forward dataset version %s and a persistent identifier', + async (version) => { + const repository = {} + repository.getFileCitationByFormat = jest.fn().mockResolvedValue('citation') + const sut = new GetFileCitationByFormat(repository) + const fileId = 'doi:10.5072/FK2/TEST/FILE' + + await expect(sut.execute(fileId, FileCitationFormat.ENDNOTE, version)).resolves.toBe( + 'citation' + ) + expect(repository.getFileCitationByFormat).toHaveBeenCalledWith( + fileId, + FileCitationFormat.ENDNOTE, + version + ) } )