From 2f1ce3b67bd79cf5036edd6f177878324def03ae Mon Sep 17 00:00:00 2001 From: Nick Phura Date: Tue, 16 Jan 2024 12:26:02 -0800 Subject: [PATCH] Remove source_id from submission. Make uuid a source submitted field for submission. make source_id optional for submission_features. --- api/src/openapi/root-api-doc.ts | 2 +- .../submission/published.test.ts | 2 -- .../submission/reviewed.test.ts | 2 -- .../submission/unreviewed.test.ts | 2 -- .../submission/{submissionId}/index.test.ts | 1 - api/src/paths/submission/intake.test.ts | 1 - api/src/paths/submission/intake.ts | 5 ++-- .../submission-repository.test.ts | 12 --------- api/src/repositories/submission-repository.ts | 25 +++++++++---------- api/src/services/artifact-service.ts | 3 +-- api/src/services/submission-service.test.ts | 12 --------- api/src/services/submission-service.ts | 6 ++--- api/src/utils/file-utils.ts | 14 +++-------- .../20231109000001_feature_tables.ts | 2 +- .../src/migrations/release.0.8.0/biohub.sql | 6 ++--- 15 files changed, 26 insertions(+), 69 deletions(-) diff --git a/api/src/openapi/root-api-doc.ts b/api/src/openapi/root-api-doc.ts index a1581b98..12e65e3b 100644 --- a/api/src/openapi/root-api-doc.ts +++ b/api/src/openapi/root-api-doc.ts @@ -144,7 +144,7 @@ export const rootAPIDoc = { SubmissionFeature: { title: 'BioHub Data Submission Feature', type: 'object', - required: ['id', 'type', 'properties', 'features'], + required: ['type', 'properties', 'features'], properties: { id: { description: 'Unique id of the feature', diff --git a/api/src/paths/administrative/submission/published.test.ts b/api/src/paths/administrative/submission/published.test.ts index 5dd40d03..5d78be3e 100644 --- a/api/src/paths/administrative/submission/published.test.ts +++ b/api/src/paths/administrative/submission/published.test.ts @@ -51,7 +51,6 @@ describe('getPublishedSubmissionsForAdmins', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', system_user_id: 3, security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', @@ -71,7 +70,6 @@ describe('getPublishedSubmissionsForAdmins', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '123', system_user_id: 3, security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', diff --git a/api/src/paths/administrative/submission/reviewed.test.ts b/api/src/paths/administrative/submission/reviewed.test.ts index 4cefb051..7059ed01 100644 --- a/api/src/paths/administrative/submission/reviewed.test.ts +++ b/api/src/paths/administrative/submission/reviewed.test.ts @@ -51,7 +51,6 @@ describe('getReviewedSubmissionsForAdmins', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', publish_timestamp: '2023-12-12', @@ -71,7 +70,6 @@ describe('getReviewedSubmissionsForAdmins', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, diff --git a/api/src/paths/administrative/submission/unreviewed.test.ts b/api/src/paths/administrative/submission/unreviewed.test.ts index d5d1ed28..c7d590b1 100644 --- a/api/src/paths/administrative/submission/unreviewed.test.ts +++ b/api/src/paths/administrative/submission/unreviewed.test.ts @@ -51,7 +51,6 @@ describe('getUnreviewedSubmissionsForAdmins', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: null, submitted_timestamp: '2023-12-12', publish_timestamp: '2023-12-12', @@ -71,7 +70,6 @@ describe('getUnreviewedSubmissionsForAdmins', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '321', security_review_timestamp: null, submitted_timestamp: '2023-12-12', publish_timestamp: '2023-12-12', diff --git a/api/src/paths/administrative/submission/{submissionId}/index.test.ts b/api/src/paths/administrative/submission/{submissionId}/index.test.ts index ee6d0dd0..ebd5f357 100644 --- a/api/src/paths/administrative/submission/{submissionId}/index.test.ts +++ b/api/src/paths/administrative/submission/{submissionId}/index.test.ts @@ -51,7 +51,6 @@ describe('patchSubmissionRecord', () => { const mockSubmissionRecord: SubmissionRecord = { submission_id: 3, uuid: '999-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', publish_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', diff --git a/api/src/paths/submission/intake.test.ts b/api/src/paths/submission/intake.test.ts index 45b34468..107ffbcc 100644 --- a/api/src/paths/submission/intake.test.ts +++ b/api/src/paths/submission/intake.test.ts @@ -111,7 +111,6 @@ describe('intake', () => { .resolves({ submission_id: submissionId, uuid: '123-456-789', - source_id: '123', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, diff --git a/api/src/paths/submission/intake.ts b/api/src/paths/submission/intake.ts index 2000dd18..f40b85f3 100644 --- a/api/src/paths/submission/intake.ts +++ b/api/src/paths/submission/intake.ts @@ -44,6 +44,7 @@ POST.apiDoc = { properties: { id: { description: 'Unique id of the submission.', + format: 'uuid', type: 'string' }, name: { @@ -127,7 +128,7 @@ export function submissionIntake(): RequestHandler { ]); } - const submissionSourceId = req.body.id; + const submissionUuid = req.body.id; const submissionName = req.body.name; const submissionDescription = req.body.description; @@ -149,7 +150,7 @@ export function submissionIntake(): RequestHandler { // insert the submission record const submissionRecord = await submissionService.insertSubmissionRecordWithPotentialConflict( - submissionSourceId, + submissionUuid, submissionName, submissionDescription, serviceClientSystemUser.system_user_id, diff --git a/api/src/repositories/submission-repository.test.ts b/api/src/repositories/submission-repository.test.ts index 285525e1..a3731729 100644 --- a/api/src/repositories/submission-repository.test.ts +++ b/api/src/repositories/submission-repository.test.ts @@ -916,7 +916,6 @@ describe('SubmissionRepository', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '123', security_review_timestamp: null, submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -933,7 +932,6 @@ describe('SubmissionRepository', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '456', security_review_timestamp: null, submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -971,7 +969,6 @@ describe('SubmissionRepository', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '123', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -988,7 +985,6 @@ describe('SubmissionRepository', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1026,7 +1022,6 @@ describe('SubmissionRepository', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1043,7 +1038,6 @@ describe('SubmissionRepository', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '456', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1080,7 +1074,6 @@ describe('SubmissionRepository', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '123', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1098,7 +1091,6 @@ describe('SubmissionRepository', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '456', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1117,7 +1109,6 @@ describe('SubmissionRepository', () => { { submission_id: 3, uuid: '999-456-123', - source_id: '789', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1381,7 +1372,6 @@ describe('SubmissionRepository', () => { const mockSubmissionRecord: SubmissionRecord = { submission_id: 1, uuid: '123-456-789', - source_id: '123', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1418,7 +1408,6 @@ describe('SubmissionRepository', () => { const mockSubmissionRecord: SubmissionRecord = { submission_id: 1, uuid: '123-456-789', - source_id: '456', security_review_timestamp: null, submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1701,7 +1690,6 @@ describe('SubmissionRepository', () => { const mockResponse: SubmissionRecordPublished = { submission_id: 1, uuid: 'string', - source_id: '123', security_review_timestamp: null, publish_timestamp: 'string', submitted_timestamp: 'string', diff --git a/api/src/repositories/submission-repository.ts b/api/src/repositories/submission-repository.ts index fcf371f2..fbb63fa8 100644 --- a/api/src/repositories/submission-repository.ts +++ b/api/src/repositories/submission-repository.ts @@ -22,7 +22,7 @@ export interface IDatasetsForReview { } export interface ISubmissionFeature { - id: string; + id: string | null; type: string; properties: Record; features: ISubmissionFeature[]; @@ -77,7 +77,7 @@ export const SubmissionFeatureRecord = z.object({ uuid: z.string(), submission_id: z.number(), feature_type_id: z.number(), - source_id: z.string(), + source_id: z.string().nullable(), data: z.record(z.any()), parent_submission_feature_id: z.number().nullable(), record_effective_date: z.string(), @@ -256,7 +256,6 @@ export interface ISubmissionObservationRecord { export const SubmissionRecord = z.object({ submission_id: z.number(), uuid: z.string(), - source_id: z.string(), security_review_timestamp: z.string().nullable(), submitted_timestamp: z.string(), system_user_id: z.number(), @@ -370,7 +369,7 @@ export class SubmissionRepository extends BaseRepository { /** * Insert a new submission record. * - * @param {string} sourceId + * @param {string} uuid * @param {string} name * @param {string} description * @param {string} userIdentifier @@ -378,7 +377,7 @@ export class SubmissionRepository extends BaseRepository { * @memberof SubmissionRepository */ async insertSubmissionRecordWithPotentialConflict( - sourceId: string, + uuid: string, name: string, description: string, systemUserId: number, @@ -386,14 +385,14 @@ export class SubmissionRepository extends BaseRepository { ): Promise { const sqlStatement = SQL` INSERT INTO submission ( - source_id, + uuid, submitted_timestamp, name, description, system_user_id, source_system ) VALUES ( - ${sourceId}, + ${uuid}, now(), ${name}, ${description}, @@ -420,7 +419,7 @@ export class SubmissionRepository extends BaseRepository { * Insert a new submission feature record. * * @param {number} submissionId - * @param {string} sourceId + * @param {(string | null)} featureSourceId * @param {string} featureTypeName * @param {ISubmissionFeature['properties']} featureProperties * @return {*} {Promise<{ submission_feature_id: number }>} @@ -428,7 +427,7 @@ export class SubmissionRepository extends BaseRepository { */ async insertSubmissionFeatureRecord( submissionId: number, - featureSourceId: string, + featureSourceId: string | null, featureTypeName: string, featureProperties: ISubmissionFeature['properties'] ): Promise<{ submission_feature_id: number }> { @@ -1220,7 +1219,7 @@ export class SubmissionRepository extends BaseRepository { const sqlStatement = SQL` WITH w_unique_submissions as ( SELECT - DISTINCT ON (submission.source_id) submission.*, + DISTINCT ON (submission.uuid) submission.*, submission_feature.feature_type_id as root_feature_type_id, feature_type.name as root_feature_type_name, ${SECURITY_APPLIED_STATUS.PENDING} as security @@ -1239,7 +1238,7 @@ export class SubmissionRepository extends BaseRepository { AND submission_feature.parent_submission_feature_id IS NULL ORDER BY - submission.source_id, submission.submission_id DESC + submission.uuid, submission.submission_id DESC ) SELECT * @@ -1263,7 +1262,7 @@ export class SubmissionRepository extends BaseRepository { const sqlStatement = SQL` WITH w_unique_submissions as ( SELECT - DISTINCT ON (submission.source_id) submission.*, + DISTINCT ON (submission.uuid) submission.*, submission_feature.feature_type_id as root_feature_type_id, feature_type.name as root_feature_type_name, CASE @@ -1297,7 +1296,7 @@ export class SubmissionRepository extends BaseRepository { submission_feature.feature_type_id, feature_type.name ORDER BY - submission.source_id, submission.submission_id DESC + submission.uuid, submission.submission_id DESC ) SELECT * diff --git a/api/src/services/artifact-service.ts b/api/src/services/artifact-service.ts index 1300ccee..153d21fd 100644 --- a/api/src/services/artifact-service.ts +++ b/api/src/services/artifact-service.ts @@ -68,8 +68,7 @@ export class ArtifactService extends DBService { // Generate S3 key const artifactS3Key = generateSubmissionFeatureS3FileKey({ submissionId: artifactFeatureSubmission.submission_id, - submissionFeatureId: artifactFeatureSubmission.submission_feature_id, - artifactId: artifactFeatureSubmission.source_id // TODO is this needed. The submission_feature_id should be enough. + submissionFeatureId: artifactFeatureSubmission.submission_feature_id }); defaultLog.debug({ label: 'uploadSubmissionFeatureArtifact', message: 'S3 key', artifactS3Key }); diff --git a/api/src/services/submission-service.test.ts b/api/src/services/submission-service.test.ts index a9808443..4cbcff26 100644 --- a/api/src/services/submission-service.test.ts +++ b/api/src/services/submission-service.test.ts @@ -62,7 +62,6 @@ describe('SubmissionService', () => { const mockSubmissionRecord: SubmissionRecord = { submission_id: 1, uuid: '123-456-789', - source_id: '123', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -915,7 +914,6 @@ describe('SubmissionService', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: null, submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -935,7 +933,6 @@ describe('SubmissionService', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -975,7 +972,6 @@ describe('SubmissionService', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: null, submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -995,7 +991,6 @@ describe('SubmissionService', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1035,7 +1030,6 @@ describe('SubmissionService', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: null, submitted_timestamp: '2023-12-12', source_system: 'SIMS', @@ -1055,7 +1049,6 @@ describe('SubmissionService', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '123', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1097,7 +1090,6 @@ describe('SubmissionService', () => { const mockResponse: SubmissionRecordWithSecurity = { submission_id: 1, uuid: 'string', - source_id: '321', security_review_timestamp: null, submitted_timestamp: 'string', system_user_id: 3, @@ -1130,7 +1122,6 @@ describe('SubmissionService', () => { { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1151,7 +1142,6 @@ describe('SubmissionService', () => { { submission_id: 2, uuid: '789-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1172,7 +1162,6 @@ describe('SubmissionService', () => { { submission_id: 3, uuid: '999-456-123', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, @@ -1385,7 +1374,6 @@ describe('SubmissionService', () => { const mockSubmissionRecord: SubmissionRecord = { submission_id: 1, uuid: '123-456-789', - source_id: '321', security_review_timestamp: '2023-12-12', submitted_timestamp: '2023-12-12', system_user_id: 3, diff --git a/api/src/services/submission-service.ts b/api/src/services/submission-service.ts index dab587f6..20d911fe 100644 --- a/api/src/services/submission-service.ts +++ b/api/src/services/submission-service.ts @@ -68,7 +68,7 @@ export class SubmissionService extends DBService { * Insert a new submission record, returning the record having the matching UUID if it already exists * in the database. * - * @param {string} sourceId + * @param {string} uuid * @param {string} name * @param {string} description * @param {number} systemUserId @@ -77,14 +77,14 @@ export class SubmissionService extends DBService { * @memberof SubmissionService */ async insertSubmissionRecordWithPotentialConflict( - sourceId: string, + uuid: string, name: string, description: string, systemUserId: number, systemUserIdentifier: string ): Promise { return this.submissionRepository.insertSubmissionRecordWithPotentialConflict( - sourceId, + uuid, name, description, systemUserId, diff --git a/api/src/utils/file-utils.ts b/api/src/utils/file-utils.ts index 1ad905fd..198beeee 100644 --- a/api/src/utils/file-utils.ts +++ b/api/src/utils/file-utils.ts @@ -18,7 +18,6 @@ export interface IDatasetS3FileKey { export interface IArtifactS3FileKey { submissionId: number; submissionFeatureId: number; - artifactId: string; } export interface IQueueS3FileKey { @@ -273,16 +272,9 @@ export async function getObjectMeta(key: string): Promise { * @return {*} */ export function generateSubmissionFeatureS3FileKey(options: IArtifactS3FileKey) { - const keyParts: (string | number)[] = []; - - keyParts.push(_getS3KeyPrefix()); - keyParts.push('submissions'); - keyParts.push(options.submissionId); - keyParts.push('features'); - keyParts.push(options.submissionFeatureId); - keyParts.push(options.artifactId); - - return keyParts.filter(Boolean).join('/'); + return [_getS3KeyPrefix(), 'submissions', options.submissionId, 'features', options.submissionFeatureId] + .filter(Boolean) + .join('/'); } /** diff --git a/database/src/migrations/20231109000001_feature_tables.ts b/database/src/migrations/20231109000001_feature_tables.ts index 9d35186e..1bfef33b 100644 --- a/database/src/migrations/20231109000001_feature_tables.ts +++ b/database/src/migrations/20231109000001_feature_tables.ts @@ -24,7 +24,7 @@ export async function up(knex: Knex): Promise { uuid uuid DEFAULT public.gen_random_uuid(), submission_id integer NOT NULL, feature_type_id integer NOT NULL, - source_id varchar(100) NOT NULL, + source_id varchar(200), data jsonb NOT NULL, parent_submission_feature_id integer, record_effective_date date DEFAULT now() NOT NULL, diff --git a/database/src/migrations/release.0.8.0/biohub.sql b/database/src/migrations/release.0.8.0/biohub.sql index 3873118a..7923286c 100644 --- a/database/src/migrations/release.0.8.0/biohub.sql +++ b/database/src/migrations/release.0.8.0/biohub.sql @@ -28,8 +28,7 @@ COMMENT ON TABLE audit_log IS 'Holds record level audit log data for the entire CREATE TABLE submission( submission_id integer GENERATED ALWAYS AS IDENTITY (START WITH 1 INCREMENT BY 1), - uuid uuid DEFAULT public.gen_random_uuid(), - source_id varchar(100) NOT NULL, + uuid uuid NOT NULL, system_user_id integer NOT NULL, source_system varchar(100) NOT NULL, security_review_timestamp timestamptz(6), @@ -47,8 +46,7 @@ CREATE TABLE submission( ); COMMENT ON COLUMN submission.submission_id IS 'System generated surrogate primary key identifier.'; -COMMENT ON COLUMN submission.uuid IS 'The globally unique identifier for this submission as supplied by BioHub.'; -COMMENT ON COLUMN submission.source_id IS 'The unique identifier for the submission as supplied by the source system. May not be unique globally or within BioHub.'; +COMMENT ON COLUMN submission.uuid IS 'The globally unique identifier for this submission as supplied by the source system.'; COMMENT ON COLUMN submission.system_user_id IS 'Foreign key to the system_user table.'; COMMENT ON COLUMN submission.source_system IS 'The name of the submitting system.'; COMMENT ON COLUMN submission.security_review_timestamp IS 'The timestamp of when the security review of the submission was completed. Null indicates the security review has not been completed.';