From 28495e878caf6fc2f0e86d59bde5e987b1b24ff0 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Mon, 31 Mar 2025 02:48:55 -0400 Subject: [PATCH 1/3] feat: update restrict extension --- docs/useCases.md | 12 ++++- src/files/domain/dtos/RestrictFileDTO.ts | 5 ++ .../domain/repositories/IFilesRepository.ts | 4 +- src/files/domain/useCases/RestrictFile.ts | 12 +++-- .../infra/repositories/FilesRepository.ts | 16 +++++-- test/functional/files/RestrictFile.test.ts | 46 ++++++++++++++----- .../integration/files/FilesRepository.test.ts | 15 +++++- test/unit/files/RestrictFile.test.ts | 11 ++++- 8 files changed, 94 insertions(+), 27 deletions(-) create mode 100644 src/files/domain/dtos/RestrictFileDTO.ts diff --git a/docs/useCases.md b/docs/useCases.md index cd21b618..9f19c5cd 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1376,7 +1376,7 @@ The use case returns a number, which is the identifier of the new file. #### Restrict or Unrestrict a File -Restrict or unrestrict an existing file. +Restrict or unrestrict an existing file, given a [RestrictFileDTO](../src/users/domain/dtos/RestrictFileDTO.ts) ##### Example call: @@ -1386,8 +1386,13 @@ import { restrictFile } from '@iqss/dataverse-client-javascript' /* ... */ const fileId = 12345 +const restrictFileDTO = { + restrict: true, + enableAccessRequest: boolean, + termsOfAccess: string +} -restrictFile.execute(fileId, true) +restrictFile.execute(fileId, restrictFileDTO) /* ... */ ``` @@ -1395,6 +1400,9 @@ restrictFile.execute(fileId, true) _See [use case](../src/files/domain/useCases/RestrictFile.ts) implementation_. The `fileId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers. +If restrict is false then enableAccessRequest and termsOfAccess are ignored +If restrict is true and enableAccessRequest is false then termsOfAccess is required. +The enableAccessRequest and termsOfAccess are applied to the Draft version of the Dataset and affect all of the restricted files in said Draft version. ## Metadata Blocks diff --git a/src/files/domain/dtos/RestrictFileDTO.ts b/src/files/domain/dtos/RestrictFileDTO.ts new file mode 100644 index 00000000..906a3dc3 --- /dev/null +++ b/src/files/domain/dtos/RestrictFileDTO.ts @@ -0,0 +1,5 @@ +export interface RestrictFileDTO { + restrict: boolean + enableAccessRequest?: boolean + termsOfAccess?: string +} diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index 476810e1..e18d8d6a 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -9,6 +9,7 @@ import { Dataset } from '../../../datasets' import { FileUploadDestination } from '../models/FileUploadDestination' import { UploadedFileDTO } from '../dtos/UploadedFileDTO' import { UpdateFileMetadataDTO } from '../dtos/UpdateFileMetadataDTO' +import { RestrictFileDTO } from '../dtos/RestrictFileDTO' export interface IFilesRepository { getDatasetFiles( @@ -65,7 +66,8 @@ export interface IFilesRepository { replaceFile(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise - restrictFile(fileId: number | string, restrict: boolean): Promise + restrictFile(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise + updateFileMetadata( fileId: number | string, updateFileMetadataDTO: UpdateFileMetadataDTO diff --git a/src/files/domain/useCases/RestrictFile.ts b/src/files/domain/useCases/RestrictFile.ts index c01d66b7..5760ab12 100644 --- a/src/files/domain/useCases/RestrictFile.ts +++ b/src/files/domain/useCases/RestrictFile.ts @@ -1,7 +1,8 @@ import { IFilesRepository } from '../repositories/IFilesRepository' import { UseCase } from '../../../core/domain/useCases/UseCase' - -export class RestrictFile implements UseCase { +import { RestrictFileDTO } from '../dtos/RestrictFileDTO' +// https://github.com/IQSS/dataverse/pull/11349/files +export class RestrictFile implements UseCase { constructor(private readonly filesRepository: IFilesRepository) {} /** @@ -9,10 +10,11 @@ export class RestrictFile implements UseCase { * More detailed information about the file restriction behavior can be found in https://guides.dataverse.org/en/latest/api/native-api.html#restrict-files * * @param {number | string} [fileId] - The File identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers). - * @param {boolean} [restrict] - A boolean value that indicates whether the file should be restricted or unrestricted. + * @param {RestrictFileDTO} [restrictFileDTO] - The DTO containing the file restriction information. * @returns {Promise} -This method does not return anything upon successful completion. */ - async execute(fileId: number | string, restrict: boolean): Promise { - return await this.filesRepository.restrictFile(fileId, restrict) + + async execute(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise { + return await this.filesRepository.restrictFile(fileId, restrictFileDTO) } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index d30494f6..7e8bc96d 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -21,6 +21,7 @@ import { transformUploadDestinationsResponseToUploadDestination } from './transf import { UploadedFileDTO } from '../../domain/dtos/UploadedFileDTO' import { UpdateFileMetadataDTO } from '../../domain/dtos/UpdateFileMetadataDTO' import { ApiConstants } from '../../../core/infra/repositories/ApiConstants' +import { RestrictFileDTO } from '../../domain/dtos/RestrictFileDTO' export interface GetFilesQueryParams { includeDeaccessioned: boolean @@ -346,9 +347,18 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { }) } - public async restrictFile(fileId: number | string, restrict: boolean): Promise { - return this.doPut(this.buildApiEndpoint(this.filesResourceName, 'restrict', fileId), restrict) - .then(() => undefined) + public async restrictFile( + fileId: number | string, + restrictFileDTO: RestrictFileDTO + ): Promise { + return this.doPut( + this.buildApiEndpoint(this.filesResourceName, 'restrict', fileId), + restrictFileDTO + ) + .then((response) => { + console.log('test', response.data.data.message) + return response.data.data.message + }) .catch((error) => { throw error }) diff --git a/test/functional/files/RestrictFile.test.ts b/test/functional/files/RestrictFile.test.ts index 39cd7d52..f7107922 100644 --- a/test/functional/files/RestrictFile.test.ts +++ b/test/functional/files/RestrictFile.test.ts @@ -58,32 +58,36 @@ describe('execute', () => { try { const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) - await restrictFile.execute(datasetFiles.files[0].id, true) + await restrictFile.execute(datasetFiles.files[0].id, { restrict: true }) } catch (error) { - throw new Error('File should be deleted') + throw new Error('File should be restricted') } finally { const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId) - expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(true) - // Unrestrict the file for the next test - await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, false) + await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, { restrict: false }) } }) test('should succesfully unrestrict a file', async () => { try { const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) - - await restrictFile.execute(datasetFiles.files[0].id, true) - - await restrictFile.execute(datasetFiles.files[0].id, false) + await restrictFile.execute(datasetFiles.files[0].id, { restrict: true }) } catch (error) { - throw new Error('File should be deleted') + throw new Error('File should be restricted') } finally { const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId) + expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(true) + } - expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(false) + try { + const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId) + await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, { restrict: false }) + } catch (error) { + throw new Error('File should be unrestricted') + } finally { + const datasetFilesAfterUnrestriction = await getDatasetFiles.execute(testDatasetIds.numericId) + expect(datasetFilesAfterUnrestriction.files[0].restricted).toEqual(false) } }) @@ -93,7 +97,7 @@ describe('execute', () => { const nonExistentFileId = 5 try { - await restrictFile.execute(nonExistentFileId, true) + await restrictFile.execute(nonExistentFileId, { restrict: true }) throw new Error('Use case should throw an error') } catch (error) { writeError = error as WriteError @@ -105,4 +109,22 @@ describe('execute', () => { ) } }) + + test('should throw an error when the terms of use is empty while enableAccess is false', async () => { + let caughtError: unknown + try { + const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) + await restrictFile.execute(datasetFiles.files[0].id, { + restrict: true, + enableAccessRequest: false + }) + } catch (error) { + caughtError = error + } + + expect(caughtError).toBeInstanceOf(WriteError) + expect((caughtError as WriteError).message).toEqual( + 'There was an error when writing the resource. Reason was: [409] Terms of Use and Access are invalid. You must enable request access or add terms of access in datasets with restricted files.' + ) + }) }) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 262e1d9f..efbcbf4d 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -41,6 +41,7 @@ import { deleteCollectionViaApi, setStorageDriverViaApi } from '../../testHelpers/collections/collectionHelper' +import { RestrictFileDTO } from '../../../src/files/domain/dtos/RestrictFileDTO' describe('FilesRepository', () => { const sut: FilesRepository = new FilesRepository() @@ -766,13 +767,23 @@ describe('FilesRepository', () => { describe('restrictFile', () => { let restrictFileDatasetIds: CreatedDatasetIdentifiers const testTextFile1Name = 'test-file-1.txt' + const restrictFileDTO: RestrictFileDTO = { + restrict: true, + enableAccessRequest: true, + termsOfAccess: 'This file is restricted for testing purposes' + } + + const unrestrictFileDTO: RestrictFileDTO = { + restrict: false, + termsOfAccess: 'This file is restricted for testing purposes' + } const setFileToRestricted = async (fileId: number) => { - await sut.restrictFile(fileId, true) + await sut.restrictFile(fileId, restrictFileDTO) } const setFileToUnrestricted = async (fileId: number) => { - await sut.restrictFile(fileId, false) + await sut.restrictFile(fileId, unrestrictFileDTO) } beforeEach(async () => { diff --git a/test/unit/files/RestrictFile.test.ts b/test/unit/files/RestrictFile.test.ts index b240809e..5684af71 100644 --- a/test/unit/files/RestrictFile.test.ts +++ b/test/unit/files/RestrictFile.test.ts @@ -1,15 +1,22 @@ import { WriteError } from '../../../src' +import { RestrictFileDTO } from '../../../src/files/domain/dtos/RestrictFileDTO' import { IFilesRepository } from '../../../src/files/domain/repositories/IFilesRepository' import { RestrictFile } from '../../../src/files/domain/useCases/RestrictFile' describe('execute', () => { + const restrictFileDTO: RestrictFileDTO = { + restrict: true, + enableAccessRequest: true, + termsOfAccess: 'This file is restricted for testing purposes' + } + test('should return undefined when repository call is successful', async () => { const filesRepositoryStub: IFilesRepository = {} as IFilesRepository filesRepositoryStub.restrictFile = jest.fn().mockResolvedValue(undefined) const sut = new RestrictFile(filesRepositoryStub) - const actual = await sut.execute(1, true) + const actual = await sut.execute(1, restrictFileDTO) expect(actual).toEqual(undefined) }) @@ -20,6 +27,6 @@ describe('execute', () => { const sut = new RestrictFile(filesRepositoryStub) - await expect(sut.execute(1, true)).rejects.toThrow(WriteError) + await expect(sut.execute(1, restrictFileDTO)).rejects.toThrow(WriteError) }) }) From 0c26c960b6e676af681ca35edb13f1b4d3ff2f6b Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Mon, 31 Mar 2025 12:26:05 -0400 Subject: [PATCH 2/3] feat: update restrict extension testcases --- docs/useCases.md | 4 +-- .../domain/repositories/IFilesRepository.ts | 2 +- src/files/domain/useCases/RestrictFile.ts | 6 ++-- .../infra/repositories/FilesRepository.ts | 3 +- test/functional/files/RestrictFile.test.ts | 32 +++++++++++++++++-- .../integration/files/FilesRepository.test.ts | 20 ++++++++++++ 6 files changed, 57 insertions(+), 10 deletions(-) diff --git a/docs/useCases.md b/docs/useCases.md index 2f2a9de5..5f077a8c 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1420,8 +1420,8 @@ import { restrictFile } from '@iqss/dataverse-client-javascript' const fileId = 12345 const restrictFileDTO = { restrict: true, - enableAccessRequest: boolean, - termsOfAccess: string + enableAccessRequest: false, + termsOfAccess: 'terms of access' } restrictFile.execute(fileId, restrictFileDTO) diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index e18d8d6a..7d23ba3f 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -66,7 +66,7 @@ export interface IFilesRepository { replaceFile(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise - restrictFile(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise + restrictFile(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise updateFileMetadata( fileId: number | string, diff --git a/src/files/domain/useCases/RestrictFile.ts b/src/files/domain/useCases/RestrictFile.ts index 5760ab12..3b082fa1 100644 --- a/src/files/domain/useCases/RestrictFile.ts +++ b/src/files/domain/useCases/RestrictFile.ts @@ -1,8 +1,8 @@ import { IFilesRepository } from '../repositories/IFilesRepository' import { UseCase } from '../../../core/domain/useCases/UseCase' import { RestrictFileDTO } from '../dtos/RestrictFileDTO' -// https://github.com/IQSS/dataverse/pull/11349/files -export class RestrictFile implements UseCase { + +export class RestrictFile implements UseCase { constructor(private readonly filesRepository: IFilesRepository) {} /** @@ -14,7 +14,7 @@ export class RestrictFile implements UseCase { * @returns {Promise} -This method does not return anything upon successful completion. */ - async execute(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise { + async execute(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise { return await this.filesRepository.restrictFile(fileId, restrictFileDTO) } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 7e8bc96d..3d41ba50 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -350,13 +350,12 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { public async restrictFile( fileId: number | string, restrictFileDTO: RestrictFileDTO - ): Promise { + ): Promise { return this.doPut( this.buildApiEndpoint(this.filesResourceName, 'restrict', fileId), restrictFileDTO ) .then((response) => { - console.log('test', response.data.data.message) return response.data.data.message }) .catch((error) => { diff --git a/test/functional/files/RestrictFile.test.ts b/test/functional/files/RestrictFile.test.ts index f7107922..7f0fc32a 100644 --- a/test/functional/files/RestrictFile.test.ts +++ b/test/functional/files/RestrictFile.test.ts @@ -4,7 +4,8 @@ import { CreatedDatasetIdentifiers, restrictFile, getDatasetFiles, - WriteError + WriteError, + getDataset } from '../../../src' import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig' import { @@ -69,6 +70,32 @@ describe('execute', () => { } }) + test('should successfully restrict a file with terms of use', async () => { + try { + const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) + + await restrictFile.execute(datasetFiles.files[0].id, { + restrict: true, + enableAccessRequest: false, + termsOfAccess: 'This file is restricted for testing purposes' + }) + } catch (error) { + throw new Error('File should be restricted') + } finally { + const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId) + + expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(true) + + const dataset = await getDataset.execute(testDatasetIds.numericId) + + expect(dataset.termsOfUse.termsOfAccess.termsOfAccessForRestrictedFiles).toEqual( + 'This file is restricted for testing purposes' + ) + + await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, { restrict: false }) + } + }) + test('should succesfully unrestrict a file', async () => { try { const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) @@ -124,7 +151,8 @@ describe('execute', () => { expect(caughtError).toBeInstanceOf(WriteError) expect((caughtError as WriteError).message).toEqual( - 'There was an error when writing the resource. Reason was: [409] Terms of Use and Access are invalid. You must enable request access or add terms of access in datasets with restricted files.' + new WriteError().message + + ' Reason was: [409] Terms of Use and Access are invalid. You must enable request access or add terms of access in datasets with restricted files.' ) }) }) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index efbcbf4d..07a644ad 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -901,5 +901,25 @@ describe('FilesRepository', () => { await expect(setFileToRestricted(nonExistentFiledId)).rejects.toThrow(expectedError) }) + + test('should return error when the terms of use is empty while enableAccess is false', async () => { + const datasetFiles = await sut.getDatasetFiles( + restrictFileDatasetIds.numericId, + DatasetNotNumberedVersion.LATEST, + false, + FileOrderCriteria.NAME_AZ + ) + + const errorExpected = new WriteError( + `[409] Terms of Use and Access are invalid. You must enable request access or add terms of access in datasets with restricted files.` + ) + + await expect( + sut.restrictFile(datasetFiles.files[0].id, { + restrict: true, + enableAccessRequest: false + }) + ).rejects.toThrow(errorExpected) + }) }) }) From 6e36a70fa3df7745fb14365e0eec24cee55d62db Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Mon, 31 Mar 2025 17:07:14 -0400 Subject: [PATCH 3/3] fix: add enable request check --- .../integration/files/FilesRepository.test.ts | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 07a644ad..98cde2b5 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -42,9 +42,11 @@ import { setStorageDriverViaApi } from '../../testHelpers/collections/collectionHelper' import { RestrictFileDTO } from '../../../src/files/domain/dtos/RestrictFileDTO' +import { DatasetsRepository } from '../../../src/datasets/infra/repositories/DatasetsRepository' describe('FilesRepository', () => { const sut: FilesRepository = new FilesRepository() + const sutDataset: DatasetsRepository = new DatasetsRepository() let testDatasetIds: CreatedDatasetIdentifiers @@ -773,10 +775,7 @@ describe('FilesRepository', () => { termsOfAccess: 'This file is restricted for testing purposes' } - const unrestrictFileDTO: RestrictFileDTO = { - restrict: false, - termsOfAccess: 'This file is restricted for testing purposes' - } + const unrestrictFileDTO: RestrictFileDTO = { restrict: false } const setFileToRestricted = async (fileId: number) => { await sut.restrictFile(fileId, restrictFileDTO) @@ -797,11 +796,15 @@ describe('FilesRepository', () => { }) }) - afterEach(async () => { - await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId) - }) + test('should successfully restrict a file enabling access request', async () => { + await publishDatasetViaApi(restrictFileDatasetIds.numericId).catch(() => { + throw new Error('Error while publishing test Dataset') + }) + + await waitForNoLocks(restrictFileDatasetIds.numericId, 10).catch(() => { + throw new Error('Error while waiting for no locks') + }) - test('should successfully restrict a file', async () => { const datasetFiles = await sut.getDatasetFiles( restrictFileDatasetIds.numericId, DatasetNotNumberedVersion.LATEST, @@ -820,10 +823,22 @@ describe('FilesRepository', () => { FileOrderCriteria.NAME_AZ ) - expect(datasetFilesAfterRestrict.files[0].restricted).toEqual(true) + expect(datasetFilesAfterRestrict.files[0].restricted).toEqual(restrictFileDTO.restrict) - // Unrestrict the file Just in case to avoid conflicts with other tests - await setFileToUnrestricted(datasetFiles.files[0].id) + const dataset = await sutDataset.getDataset( + restrictFileDatasetIds.numericId, + DatasetNotNumberedVersion.LATEST, + false, + false + ) + expect(datasetFilesAfterRestrict.files[0].fileAccessRequest).toEqual( + restrictFileDTO.enableAccessRequest + ) + expect(dataset.termsOfUse.termsOfAccess.termsOfAccessForRestrictedFiles).toEqual( + restrictFileDTO.termsOfAccess + ) + + await deletePublishedDatasetViaApi(restrictFileDatasetIds.persistentId) }) test('should successfully unrestrict a file', async () => { @@ -857,6 +872,8 @@ describe('FilesRepository', () => { ) expect(datasetFilesAfterUnrestrict.files[0].restricted).toEqual(false) + + await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId) }) test('should return error when file was already restricted', async () => { @@ -877,6 +894,8 @@ describe('FilesRepository', () => { // Unrestrict the file Just in case to avoid conflicts with other tests await setFileToUnrestricted(datasetFiles.files[0].id) + + await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId) }) test('should return error when files was already unrestricted', async () => { @@ -892,6 +911,8 @@ describe('FilesRepository', () => { ) await expect(setFileToUnrestricted(datasetFiles.files[0].id)).rejects.toThrow(expectedError) + + await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId) }) test('should return error when file does not exist', async () => { @@ -900,6 +921,8 @@ describe('FilesRepository', () => { ) await expect(setFileToRestricted(nonExistentFiledId)).rejects.toThrow(expectedError) + + await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId) }) test('should return error when the terms of use is empty while enableAccess is false', async () => { @@ -920,6 +943,8 @@ describe('FilesRepository', () => { enableAccessRequest: false }) ).rejects.toThrow(errorExpected) + + await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId) }) }) })