Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add datasetVersionId support in getFile use case #128

Merged
merged 1 commit into from
Feb 26, 2024
Merged
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
1 change: 1 addition & 0 deletions src/datasets/domain/models/DatasetNotNumberedVersion.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export enum DatasetNotNumberedVersion {
DRAFT = ':draft',
LATEST = ':latest',
LATEST_PUBLISHED = ':latest-published',
}
16 changes: 1 addition & 15 deletions src/files/infra/repositories/FilesRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { FileSearchCriteria, FileOrderCriteria } from '../../domain/models/FileC
import { FileCounts } from '../../domain/models/FileCounts';
import { transformFileCountsResponseToFileCounts } from './transformers/fileCountsTransformers';
import { FileDownloadSizeMode } from '../../domain/models/FileDownloadSizeMode';
import { DatasetNotNumberedVersion } from '../../../datasets';

export interface GetFilesQueryParams {
includeDeaccessioned: boolean;
Expand Down Expand Up @@ -146,7 +145,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository {
}

public async getFile(fileId: number | string, datasetVersionId: string): Promise<File> {
return this.doGet(this.getFileEndpoint(fileId, datasetVersionId), true)
return this.doGet(this.buildApiEndpoint(this.filesResourceName, `versions/${datasetVersionId}`, fileId), true)
.then((response) => transformFileResponseToFile(response))
.catch((error) => {
throw error;
Expand All @@ -169,19 +168,6 @@ export class FilesRepository extends ApiRepository implements IFilesRepository {
});
}

private getFileEndpoint(fileId: number | string, datasetVersionId: string): string {
if (datasetVersionId === DatasetNotNumberedVersion.DRAFT) {
return this.buildApiEndpoint(this.filesResourceName, 'draft', fileId);
}
if (datasetVersionId === DatasetNotNumberedVersion.LATEST) {
return this.buildApiEndpoint(this.filesResourceName, '', fileId);
}
// TODO: Implement once it is supported by the API https://github.com/IQSS/dataverse/issues/10280
throw new Error(
`Requesting a file by its dataset version is not yet supported. Requested version: ${datasetVersionId}. Please try using the :latest or :draft version instead.`,
);
}

private applyFileSearchCriteriaToQueryParams(
queryParams: GetFilesQueryParams | GetFilesTotalDownloadSizeQueryParams,
fileSearchCriteria: FileSearchCriteria,
Expand Down
109 changes: 42 additions & 67 deletions test/integration/files/FilesRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@ import { ApiConfig, DataverseApiAuthMechanism } from '../../../src/core/infra/re
import { assert } from 'sinon';
import { expect } from 'chai';
import { TestConstants } from '../../testHelpers/TestConstants';
import {registerFileViaApi, uploadFileViaApi} from '../../testHelpers/files/filesHelper';
import { registerFileViaApi, uploadFileViaApi } from '../../testHelpers/files/filesHelper';
import { DatasetsRepository } from '../../../src/datasets/infra/repositories/DatasetsRepository';
import { ReadError } from '../../../src/core/domain/repositories/ReadError';
import { FileSearchCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria';
import { DatasetNotNumberedVersion } from '../../../src/datasets';
import { FileCounts } from '../../../src/files/domain/models/FileCounts';
import { FileDownloadSizeMode } from '../../../src';
import { fail } from 'assert';
import {deaccessionDatasetViaApi, publishDatasetViaApi, waitForNoLocks} from "../../testHelpers/datasets/datasetHelper";
import {
deaccessionDatasetViaApi,
publishDatasetViaApi,
waitForNoLocks,
} from '../../testHelpers/datasets/datasetHelper';

describe('FilesRepository', () => {
const sut: FilesRepository = new FilesRepository();
Expand All @@ -33,7 +37,9 @@ describe('FilesRepository', () => {
beforeAll(async () => {
ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.API_KEY, process.env.TEST_API_KEY);
// Uploading test file 1 with some categories
const uploadFileResponse = await uploadFileViaApi(TestConstants.TEST_CREATED_DATASET_1_ID, testTextFile1Name, { categories: [testCategoryName] })
const uploadFileResponse = await uploadFileViaApi(TestConstants.TEST_CREATED_DATASET_1_ID, testTextFile1Name, {
categories: [testCategoryName],
})
.then()
.catch((e) => {
console.log(e);
Expand Down Expand Up @@ -63,11 +69,11 @@ describe('FilesRepository', () => {
// Registering test file 1
await registerFileViaApi(uploadFileResponse.data.data.files[0].dataFile.id);
const filesSubset = await sut.getDatasetFiles(
TestConstants.TEST_CREATED_DATASET_1_ID,
latestDatasetVersionId,
false,
FileOrderCriteria.NAME_AZ,
)
TestConstants.TEST_CREATED_DATASET_1_ID,
latestDatasetVersionId,
false,
FileOrderCriteria.NAME_AZ,
);
testFileId = filesSubset.files[0].id;
testFilePersistentId = filesSubset.files[0].persistentId;
});
Expand Down Expand Up @@ -447,40 +453,28 @@ describe('FilesRepository', () => {

describe('getFile', () => {
describe('by numeric id', () => {
test('should return file when providing a valid id', async () => {
const actual = await sut.getFile(testFileId, DatasetNotNumberedVersion.LATEST);
test('should return file when providing a valid id', async () => {
const actual = await sut.getFile(testFileId, DatasetNotNumberedVersion.LATEST);

assert.match(actual.name, testTextFile1Name);
});
assert.match(actual.name, testTextFile1Name);
});

test('should return file draft when providing a valid id and version is draft', async () => {
const actual = await sut.getFile(testFileId, DatasetNotNumberedVersion.DRAFT);

assert.match(actual.name, testTextFile1Name);
});

test('should return Not Implemented Yet error when when providing a valid id and version is different than latest and draft', async () => {
// This tests can be removed once the API supports getting a file by version
test('should return error when file does not exist', async () => {
let error: ReadError = undefined;

await sut.getFile(testFileId, '1.0').catch((e) => (error = e));
await sut.getFile(nonExistentFiledId, DatasetNotNumberedVersion.LATEST).catch((e) => (error = e));

assert.match(
error.message,
`Requesting a file by its dataset version is not yet supported. Requested version: 1.0. Please try using the :latest or :draft version instead.`,
error.message,
`There was an error when reading the resource. Reason was: [404] File with ID ${nonExistentFiledId} not found.`,
);
});

test('should return error when file does not exist', async () => {
let error: ReadError = undefined;

await sut.getFile(nonExistentFiledId, DatasetNotNumberedVersion.LATEST).catch((e) => (error = e));

assert.match(
error.message,
`There was an error when reading the resource. Reason was: [400] Error attempting get the requested data file.`,
);
});
});
describe('by persistent id', () => {
test('should return file when providing a valid persistent id', async () => {
Expand All @@ -495,65 +489,46 @@ describe('FilesRepository', () => {
assert.match(actual.name, testTextFile1Name);
});

test('should return Not Implemented Yet error when when providing a valid persistent id and version is different than latest and draft', async () => {
// This tests can be removed once the API supports getting a file by version
let error: ReadError = undefined;

await sut.getFile(testFilePersistentId, '1.0').catch((e) => (error = e));

assert.match(
error.message,
`Requesting a file by its dataset version is not yet supported. Requested version: 1.0. Please try using the :latest or :draft version instead.`,
);
});

test('should return error when file does not exist', async () => {
let error: ReadError = undefined;

const nonExistentFiledPersistentId = 'nonExistentFiledPersistentId';
await sut.getFile(nonExistentFiledPersistentId, DatasetNotNumberedVersion.LATEST).catch((e) => (error = e));

assert.match(
error.message,
`There was an error when reading the resource. Reason was: [400] Error attempting get the requested data file.`,
error.message,
`There was an error when reading the resource. Reason was: [404] Datafile with Persistent ID ${nonExistentFiledPersistentId} not found.`,
);
});
});
});

describe('getFileCitation', () => {
test('should return citation when file exists', async () => {
const actualFileCitation = await sut.getFileCitation(
testFileId,
DatasetNotNumberedVersion.LATEST,
false,
);
const actualFileCitation = await sut.getFileCitation(testFileId, DatasetNotNumberedVersion.LATEST, false);
expect(typeof actualFileCitation).to.be.a('string');
});

test('should return citation when dataset is deaccessioned', async () => {
await publishDatasetViaApi(TestConstants.TEST_CREATED_DATASET_1_ID)
.then()
.catch(() => {
assert.fail('Error while publishing test Dataset');
});
.then()
.catch(() => {
assert.fail('Error while publishing test Dataset');
});

await waitForNoLocks(TestConstants.TEST_CREATED_DATASET_1_ID, 10)
.then()
.catch(() => {
assert.fail('Error while waiting for no locks');
});
.then()
.catch(() => {
assert.fail('Error while waiting for no locks');
});

await deaccessionDatasetViaApi(TestConstants.TEST_CREATED_DATASET_1_ID, '1.0')
.then()
.catch(() => {
assert.fail('Error while deaccessioning test Dataset');
});

const actualFileCitation = await sut.getFileCitation(
testFileId,
DatasetNotNumberedVersion.LATEST,
true,
);
.then()
.catch(() => {
assert.fail('Error while deaccessioning test Dataset');
});

const actualFileCitation = await sut.getFileCitation(testFileId, DatasetNotNumberedVersion.LATEST, true);
expect(typeof actualFileCitation).to.be.a('string');
});

Expand All @@ -562,8 +537,8 @@ describe('FilesRepository', () => {
await sut.getFileCitation(nonExistentFiledId, DatasetNotNumberedVersion.LATEST, false).catch((e) => (error = e));

assert.match(
error.message,
`There was an error when reading the resource. Reason was: [404] File with ID ${nonExistentFiledId} not found.`,
error.message,
`There was an error when reading the resource. Reason was: [404] File with ID ${nonExistentFiledId} not found.`,
);
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/unit/files/FilesRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ describe('FilesRepository', () => {
});
describe('getFile' , () => {
describe('by numeric id', () => {
const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/files/${testFile.id}/`;
const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/files/${testFile.id}/versions/${DatasetNotNumberedVersion.LATEST}`;
const testGetFileResponse = {
data: {
status: 'OK',
Expand Down Expand Up @@ -826,7 +826,7 @@ describe('FilesRepository', () => {
});
});
describe('by persistent id', () => {
const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/files/:persistentId/?persistentId=${TestConstants.TEST_DUMMY_PERSISTENT_ID}`;
const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/files/:persistentId/versions/${DatasetNotNumberedVersion.LATEST}?persistentId=${TestConstants.TEST_DUMMY_PERSISTENT_ID}`;
const testGetFileResponse = {
data: {
status: 'OK',
Expand Down
2 changes: 1 addition & 1 deletion test/unit/files/GetFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ describe('execute', () => {

assert.match(actualError, testReadError);
})
});
});
Loading