diff --git a/src/datasets/domain/models/Dataset.ts b/src/datasets/domain/models/Dataset.ts index e858de9e..f76f11f2 100644 --- a/src/datasets/domain/models/Dataset.ts +++ b/src/datasets/domain/models/Dataset.ts @@ -22,7 +22,12 @@ export interface DatasetVersionInfo { minorNumber: number state: DatasetVersionState createTime: Date - lastUpdateTime: Date + /** + * The timestamp of the last update to this dataset version. + * Format: ISO 8601 string (e.g., "2023-06-01T12:34:56Z"). + * Used for optimistic concurrency control to detect concurrent updates. + */ + lastUpdateTime: string releaseTime?: Date deaccessionNote?: string } diff --git a/src/datasets/domain/repositories/IDatasetsRepository.ts b/src/datasets/domain/repositories/IDatasetsRepository.ts index e78816c4..10843afa 100644 --- a/src/datasets/domain/repositories/IDatasetsRepository.ts +++ b/src/datasets/domain/repositories/IDatasetsRepository.ts @@ -54,7 +54,7 @@ export interface IDatasetsRepository { datasetId: number | string, dataset: DatasetDTO, datasetMetadataBlocks: MetadataBlock[], - internalVersionNumber?: number + sourceLastUpdateTime?: string ): Promise deaccessionDataset( datasetId: number | string, diff --git a/src/datasets/domain/useCases/UpdateDataset.ts b/src/datasets/domain/useCases/UpdateDataset.ts index ed90f4d1..26ca23b9 100644 --- a/src/datasets/domain/useCases/UpdateDataset.ts +++ b/src/datasets/domain/useCases/UpdateDataset.ts @@ -18,7 +18,7 @@ export class UpdateDataset extends DatasetWriteUseCase { * * @param {number | string} [datasetId] - The dataset identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers). * @param {DatasetDTO} [updatedDataset] - DatasetDTO object including the updated dataset metadata field values for each metadata block. - * @param {number} [internalVersionNumber] - The internal version number of the dataset. If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional internalVersionNumber parameter. This parameter must include the internal version number corresponding to the dataset version being updated. Note that internal version numbers increase sequentially with each version update. + * @param {string} [sourceLastUpdateTime] - The lastUpdateTime value from the dataset. If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional sourceLastUpdateTime parameter. This parameter must include the lastUpdateTime value corresponding to the dataset version being updated. * @returns {Promise} - This method does not return anything upon successful completion. * @throws {ResourceValidationError} - If there are validation errors related to the provided information. * @throws {ReadError} - If there are errors while reading data. @@ -27,7 +27,7 @@ export class UpdateDataset extends DatasetWriteUseCase { async execute( datasetId: number | string, updatedDataset: DatasetDTO, - internalVersionNumber?: number + sourceLastUpdateTime?: string ): Promise { const metadataBlocks = await this.getNewDatasetMetadataBlocks(updatedDataset) this.getNewDatasetValidator().validate(updatedDataset, metadataBlocks) @@ -35,7 +35,7 @@ export class UpdateDataset extends DatasetWriteUseCase { datasetId, updatedDataset, metadataBlocks, - internalVersionNumber + sourceLastUpdateTime ) } } diff --git a/src/datasets/infra/repositories/DatasetsRepository.ts b/src/datasets/infra/repositories/DatasetsRepository.ts index 1545a43d..17a77dce 100644 --- a/src/datasets/infra/repositories/DatasetsRepository.ts +++ b/src/datasets/infra/repositories/DatasetsRepository.ts @@ -252,16 +252,14 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi datasetId: string | number, dataset: DatasetDTO, datasetMetadataBlocks: MetadataBlock[], - internalVersionNumber?: number + sourceLastUpdateTime?: string ): Promise { return this.doPut( this.buildApiEndpoint(this.datasetsResourceName, `editMetadata`, datasetId), transformDatasetModelToUpdateDatasetRequestPayload(dataset, datasetMetadataBlocks), { replace: true, - ...(typeof internalVersionNumber === 'number' && { - sourceInternalVersionNumber: internalVersionNumber - }) + ...(sourceLastUpdateTime && { sourceLastUpdateTime }) } ) .then(() => undefined) diff --git a/src/datasets/infra/repositories/transformers/datasetPreviewsTransformers.ts b/src/datasets/infra/repositories/transformers/datasetPreviewsTransformers.ts index 8b60828d..3ae56800 100644 --- a/src/datasets/infra/repositories/transformers/datasetPreviewsTransformers.ts +++ b/src/datasets/infra/repositories/transformers/datasetPreviewsTransformers.ts @@ -39,7 +39,7 @@ export const transformDatasetPreviewPayloadToDatasetPreview = ( minorNumber: datasetPreviewPayload.minorVersion, state: datasetPreviewPayload.versionState as DatasetVersionState, createTime: new Date(datasetPreviewPayload.createdAt), - lastUpdateTime: new Date(datasetPreviewPayload.updatedAt), + lastUpdateTime: datasetPreviewPayload.updatedAt, ...(datasetPreviewPayload.published_at && { releaseTime: new Date(datasetPreviewPayload.published_at) }) @@ -72,7 +72,7 @@ export const transformMyDataDatasetPreviewPayloadToDatasetPreview = ( minorNumber: datasetPreviewPayload.minorVersion, state: datasetPreviewPayload.versionState as DatasetVersionState, createTime: new Date(datasetPreviewPayload.createdAt), - lastUpdateTime: new Date(datasetPreviewPayload.updatedAt), + lastUpdateTime: datasetPreviewPayload.updatedAt, ...(datasetPreviewPayload.published_at && { releaseTime: new Date(datasetPreviewPayload.published_at) }) diff --git a/src/datasets/infra/repositories/transformers/datasetTransformers.ts b/src/datasets/infra/repositories/transformers/datasetTransformers.ts index bbb4c9fc..7f186fb2 100644 --- a/src/datasets/infra/repositories/transformers/datasetTransformers.ts +++ b/src/datasets/infra/repositories/transformers/datasetTransformers.ts @@ -235,7 +235,7 @@ export const transformVersionPayloadToDataset = ( minorNumber: versionPayload.versionMinorNumber, state: versionPayload.versionState as DatasetVersionState, createTime: new Date(versionPayload.createTime), - lastUpdateTime: new Date(versionPayload.lastUpdateTime), + lastUpdateTime: versionPayload.lastUpdateTime, releaseTime: new Date(versionPayload.releaseTime), deaccessionNote: versionPayload.deaccessionNote }, diff --git a/src/files/domain/models/FileModel.ts b/src/files/domain/models/FileModel.ts index abf95d71..559396b8 100644 --- a/src/files/domain/models/FileModel.ts +++ b/src/files/domain/models/FileModel.ts @@ -30,6 +30,12 @@ export interface FileModel { tabularTags?: string[] creationDate?: string publicationDate?: string + /** + * The timestamp of the last update to this file record. + * Format: ISO 8601 string (e.g., "2023-06-01T12:34:56Z"). + * Used for optimistic concurrency control to detect concurrent updates. + */ + lastUpdateTime: string deleted: boolean tabularData: boolean fileAccessRequest?: boolean diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index 8049010c..9256d92d 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -72,7 +72,8 @@ export interface IFilesRepository { updateFileMetadata( fileId: number | string, - updateFileMetadataDTO: UpdateFileMetadataDTO + updateFileMetadataDTO: UpdateFileMetadataDTO, + sourceLastUpdateTime?: string ): Promise updateFileTabularTags( diff --git a/src/files/domain/useCases/UpdateFileMetadata.ts b/src/files/domain/useCases/UpdateFileMetadata.ts index f06c96c3..c3b62d8a 100644 --- a/src/files/domain/useCases/UpdateFileMetadata.ts +++ b/src/files/domain/useCases/UpdateFileMetadata.ts @@ -15,12 +15,18 @@ export class UpdateFileMetadata implements UseCase { * * @param {number | string} [fileId] - The file identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers). * @param {UpdateFileMetadataDTO} [updateFileMetadataDTO] - The DTO containing the metadata updates. + * @param {string} [sourceLastUpdateTime] - The lastUpdateTime value from the file. If another user updates the file metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional sourceLastUpdateTime parameter. This parameter must include the lastUpdateTime value corresponding to the file being updated. * @returns {Promise} */ async execute( fileId: number | string, - updateFileMetadataDTO: UpdateFileMetadataDTO + updateFileMetadataDTO: UpdateFileMetadataDTO, + sourceLastUpdateTime?: string ): Promise { - await this.filesRepository.updateFileMetadata(fileId, updateFileMetadataDTO) + await this.filesRepository.updateFileMetadata( + fileId, + updateFileMetadataDTO, + sourceLastUpdateTime + ) } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 3430cd4d..3d24edaf 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -369,7 +369,8 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { public async updateFileMetadata( fileId: string | number, - updateFileMetadata: UpdateFileMetadataDTO + updateFileMetadata: UpdateFileMetadataDTO, + sourceLastUpdateTime?: string ): Promise { const formData = new FormData() formData.append('jsonData', JSON.stringify(updateFileMetadata)) @@ -377,7 +378,9 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { return this.doPost( this.buildApiEndpoint(this.filesResourceName, `${fileId}/metadata`), formData, - {}, + { + ...(sourceLastUpdateTime && { sourceLastUpdateTime }) + }, ApiConstants.CONTENT_TYPE_MULTIPART_FORM_DATA ) .then(() => undefined) diff --git a/src/files/infra/repositories/transformers/FilePayload.ts b/src/files/infra/repositories/transformers/FilePayload.ts index 58afcc4a..cc7ad6b1 100644 --- a/src/files/infra/repositories/transformers/FilePayload.ts +++ b/src/files/infra/repositories/transformers/FilePayload.ts @@ -29,6 +29,7 @@ export interface FilePayload { tabularTags?: string[] creationDate?: string publicationDate?: string + lastUpdateTime: string deleted: boolean tabularData: boolean fileAccessRequest?: boolean diff --git a/src/files/infra/repositories/transformers/fileTransformers.ts b/src/files/infra/repositories/transformers/fileTransformers.ts index 5350af01..a93b7674 100644 --- a/src/files/infra/repositories/transformers/fileTransformers.ts +++ b/src/files/infra/repositories/transformers/fileTransformers.ts @@ -93,7 +93,8 @@ const transformFilePayloadToFile = (filePayload: FilePayload): FileModel => { }), ...(filePayload.dataFile.isPartOf && { isPartOf: transformPayloadToOwnerNode(filePayload.dataFile.isPartOf) - }) + }), + lastUpdateTime: filePayload.dataFile.lastUpdateTime } } diff --git a/test/integration/datasets/DatasetsRepository.test.ts b/test/integration/datasets/DatasetsRepository.test.ts index af669e7c..71b444bf 100644 --- a/test/integration/datasets/DatasetsRepository.test.ts +++ b/test/integration/datasets/DatasetsRepository.test.ts @@ -235,7 +235,6 @@ describe('DatasetsRepository', () => { false ) expect(actual.id).toBe(testDatasetIds.numericId) - expect(actual.internalVersionNumber).toBe(1) }) test('should return dataset when it is deaccessioned and includeDeaccessioned param is set', async () => { @@ -1132,8 +1131,8 @@ describe('DatasetsRepository', () => { } ]) }) - // TODO: add this test when https://github.com/IQSS/dataverse-client-javascript/issues/343 is fixed - test.skip('should throw error if trying to update an outdated internal version dataset', async () => { + + test('should throw error if sending an outdated lastUpdateTime', async () => { const testDataset = { metadataBlockValues: [ { @@ -1184,35 +1183,27 @@ describe('DatasetsRepository', () => { false, false ) - const actualCreatedDatasetInternalVersionNumber = actualCreatedDataset.internalVersionNumber - - expect(actualCreatedDataset.internalVersionNumber).toBe(1) + const firstLastUpdateTime = actualCreatedDataset.versionInfo.lastUpdateTime - // Now update the dataset and then update again with the same internal version number + // Now update the dataset and then update again with the same source last update time const updatedDsDescription = 'This is the updated description of the dataset.' testDataset.metadataBlockValues[0].fields.dsDescription[0].dsDescriptionValue = updatedDsDescription - // First update sending the correct internal version number + // Wait for 2 seconds + await new Promise((resolve) => setTimeout(resolve, 2000)) + + // First update sending the correct lastUpdateTime await sut.updateDataset( createdDataset.numericId, testDataset, [citationMetadataBlock], - actualCreatedDatasetInternalVersionNumber - ) - - const afterFirstUpdateDataset = await sut.getDataset( - createdDataset.numericId, - DatasetNotNumberedVersion.LATEST, - false, - false + firstLastUpdateTime ) - expect(afterFirstUpdateDataset.internalVersionNumber).toBe(2) - - //Now try to update again with the previous internal version number + //Now try to update again with the previous lastUpdateTime const expectedError = new WriteError( - `[400] Dataset internal version number ${actualCreatedDatasetInternalVersionNumber} is outdated` + `[400] Internal version timestamp ${firstLastUpdateTime} is outdated` ) await expect( @@ -1220,7 +1211,7 @@ describe('DatasetsRepository', () => { createdDataset.numericId, testDataset, [citationMetadataBlock], - actualCreatedDatasetInternalVersionNumber + firstLastUpdateTime ) ).rejects.toThrow(expectedError) }) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index cde53fcd..c20fd0c4 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -767,6 +767,61 @@ describe('FilesRepository', () => { errorExpected ) }) + + test('should throw error when using outdated sourceLastUpdateTime', async () => { + const newDatasetIds = await createDataset.execute(TestConstants.TEST_NEW_DATASET_DTO) + await uploadFileViaApi(newDatasetIds.numericId, testTextFile1Name) + const filesSubset = await sut.getDatasetFiles( + newDatasetIds.numericId, + latestDatasetVersionId, + false, + FileOrderCriteria.NAME_AZ + ) + const fileId = filesSubset.files[0].id + + await registerFileViaApi(fileId) + + // Fetch file to obtain initial lastUpdateTime from returned model including dataset version + const fileInfo: FileModel = (await sut.getFile( + fileId, + DatasetNotNumberedVersion.LATEST, + false, + false + )) as FileModel + + const lastUpdateTimeOne = fileInfo.lastUpdateTime + + // Wait for 2 seconds + await new Promise((resolve) => setTimeout(resolve, 2_000)) + + // First update using correct lastUpdateTime should succeed + await sut.updateFileMetadata(fileId, { description: 'First update desc.' }, lastUpdateTimeOne) + + // Refetch to get new lastUpdateTime + const fileInfoAfterFirstUpdate: FileModel = (await sut.getFile( + fileId, + DatasetNotNumberedVersion.LATEST, + false, + false + )) as FileModel + + const lastUpdateTimeTwo = fileInfoAfterFirstUpdate.lastUpdateTime + + expect(lastUpdateTimeTwo).not.toBe(lastUpdateTimeOne) + + // Wait for 2 seconds + await new Promise((resolve) => setTimeout(resolve, 2_000)) + + // Second update using stale lastUpdateTimeOne should fail + const expectedError = new WriteError( + `[400] Internal version timestamp ${lastUpdateTimeOne} is outdated` + ) + await expect( + sut.updateFileMetadata(fileId, { description: 'Second update attempt.' }, lastUpdateTimeOne) + ).rejects.toThrow(expectedError) + + await deletePublishedDatasetViaApi(newDatasetIds.persistentId) + }) }) describe('updateFileTabularTags', () => { diff --git a/test/testHelpers/datasets/datasetHelper.ts b/test/testHelpers/datasets/datasetHelper.ts index 4cb1ee79..d9f9405c 100644 --- a/test/testHelpers/datasets/datasetHelper.ts +++ b/test/testHelpers/datasets/datasetHelper.ts @@ -51,7 +51,7 @@ export const createDatasetModel = ( minorNumber: 0, state: DatasetVersionState.RELEASED, createTime: new Date(DATASET_CREATE_TIME_STR), - lastUpdateTime: new Date(DATASET_UPDATE_TIME_STR), + lastUpdateTime: DATASET_UPDATE_TIME_STR, releaseTime: new Date(DATASET_RELEASE_TIME_STR), deaccessionNote: undefined }, diff --git a/test/testHelpers/datasets/datasetPreviewHelper.ts b/test/testHelpers/datasets/datasetPreviewHelper.ts index ee162d9a..4f65c077 100644 --- a/test/testHelpers/datasets/datasetPreviewHelper.ts +++ b/test/testHelpers/datasets/datasetPreviewHelper.ts @@ -25,7 +25,7 @@ export const createDatasetPreviewModel = (): DatasetPreview => { minorNumber: 0, state: DatasetVersionState.RELEASED, createTime: new Date(DATASET_CREATE_TIME_STR), - lastUpdateTime: new Date(DATASET_UPDATE_TIME_STR), + lastUpdateTime: DATASET_UPDATE_TIME_STR, releaseTime: new Date(DATASET_RELEASE_TIME_STR) }, citation: DATASET_CITATION_HTML, diff --git a/test/testHelpers/files/filesHelper.ts b/test/testHelpers/files/filesHelper.ts index 1f0e6296..a4fce9ed 100644 --- a/test/testHelpers/files/filesHelper.ts +++ b/test/testHelpers/files/filesHelper.ts @@ -60,7 +60,8 @@ export const createFileModel = (): FileModel => { originalSize: 127426, originalName: 'originalName', tabularTags: ['tag1', 'tag2'], - publicationDate: '2023-07-11' + publicationDate: '2023-07-11', + lastUpdateTime: '2023-07-11' } } @@ -122,7 +123,8 @@ export const createFilePayload = (): FilePayload => { originalSize: 127426, originalName: 'originalName', tabularTags: ['tag1', 'tag2'], - publicationDate: '2023-07-11' + publicationDate: '2023-07-11', + lastUpdateTime: '2023-07-11' } } } diff --git a/test/unit/files/UpdateFileMetadata.test.ts b/test/unit/files/UpdateFileMetadata.test.ts index 41255e48..2175e4f2 100644 --- a/test/unit/files/UpdateFileMetadata.test.ts +++ b/test/unit/files/UpdateFileMetadata.test.ts @@ -13,7 +13,11 @@ describe('UpdateFileMetadata', () => { await sut.execute(1, testFileMetadata) - expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledWith(1, testFileMetadata) + expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledWith( + 1, + testFileMetadata, + undefined + ) expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledTimes(1) }) @@ -28,7 +32,8 @@ describe('UpdateFileMetadata', () => { expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledWith( 'doi:10.5072/FK2/HC6KTB', - testFileMetadata + testFileMetadata, + undefined ) expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledTimes(1) }) @@ -41,6 +46,10 @@ describe('UpdateFileMetadata', () => { const sut = new UpdateFileMetadata(filesRepositoryStub) await expect(sut.execute(1, testFileMetadata)).rejects.toThrow(WriteError) - expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledWith(1, testFileMetadata) + expect(filesRepositoryStub.updateFileMetadata).toHaveBeenCalledWith( + 1, + testFileMetadata, + undefined + ) }) })