Skip to content

Commit 55c234b

Browse files
committed
Rework getter setter into virtual
1 parent 80808e7 commit 55c234b

19 files changed

Lines changed: 184 additions & 126 deletions

File tree

backend/src/connectors/audit/stroom.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,23 +258,23 @@ export class StroomAuditConnector extends BaseAuditConnector {
258258
}
259259

260260
async onCreateRelease(req: Request, release: ReleaseDoc) {
261-
this.auditGenericEvent(req, `${release.modelId}:${release.semver}`)
261+
this.auditGenericEvent(req, `${release.modelId}:${release.semverString}`)
262262
}
263263

264264
async onViewRelease(req: Request, release: ReleaseDoc) {
265-
this.auditGenericEvent(req, `${release.modelId}:${release.semver}`)
265+
this.auditGenericEvent(req, `${release.modelId}:${release.semverString}`)
266266
}
267267

268268
async onViewReleases(req: Request, releases: ReleaseDoc[]) {
269269
this.auditMultipleViewEvent(
270270
req,
271-
releases.map((release) => ({ Id: `${release.modelId}:${release.semver}` })),
271+
releases.map((release) => ({ Id: `${release.modelId}:${release.semverString}` })),
272272
'release',
273273
)
274274
}
275275

276276
async onUpdateRelease(req: Request, release: ReleaseDoc) {
277-
this.auditGenericEvent(req, `${release.modelId}:${release.semver}`)
277+
this.auditGenericEvent(req, `${release.modelId}:${release.semverString}`)
278278
}
279279

280280
async onDeleteRelease(req: Request, modelId: string, semver: string) {

backend/src/migrations/020_2426_remove_orphaned_reviews_responses.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ export async function up() {
99
const deletedReviews: ReviewDoc[] = []
1010

1111
// For each deleted release, deleted the reviews associated with it
12-
for (const { modelId, semver } of deletedReleases) {
13-
const reviews = await removeReleaseReviews(modelId, semver)
12+
for (const { modelId, semverString } of deletedReleases) {
13+
const reviews = await removeReleaseReviews(modelId, semverString)
1414
deletedReviews.push(...reviews)
1515
}
1616

backend/src/migrations/025_fix_partial_020_2426.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ export async function up() {
99
const deletedReviews: ReviewDoc[] = []
1010

1111
// For each deleted release, find the already deleted reviews (from 020_2426_remove_orphaned_reviews_responses.ts) associated with it
12-
for (const { modelId, semver } of deletedReleases) {
12+
for (const { modelId, semverString } of deletedReleases) {
1313
const reviews: ReviewDoc[] = await Review.find({
1414
modelId,
15-
semver,
15+
semver: semverString,
1616
deleted: true,
1717
})
1818
deletedReviews.push(...reviews)

backend/src/models/Release.ts

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { HydratedDocument, model, ObjectId, Schema, Types } from 'mongoose'
22

3-
import { semverObjectToString, semverStringToObject } from '../services/release.js'
3+
import { semverObjectToString, semverStringToObject } from '../utils/semver.js'
44
import { SoftDeleteDocument, softDeletionPlugin } from './plugins/softDeletePlugin.js'
55

66
// This interface stores information about the properties on the base object.
@@ -10,7 +10,7 @@ export interface ReleaseInterface {
1010
modelId: string
1111
modelCardVersion: number
1212

13-
semver: string
13+
semver: SemverObject
1414
notes: string
1515

1616
minor: boolean
@@ -26,6 +26,10 @@ export interface ReleaseInterface {
2626
updatedAt: Date
2727
}
2828

29+
export interface ReleaseVirtuals {
30+
semverString: string
31+
}
32+
2933
export interface ImageNameRef {
3034
repository: string
3135
name: string
@@ -44,7 +48,12 @@ export type ImageRef = ImageTagRef | ImageDigestRef
4448
export type ReleaseDoc = HydratedDocument<
4549
Omit<ReleaseInterface, 'images'> & {
4650
images: Array<HydratedDocument<ImageTagRef>>
47-
}
51+
},
52+
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
53+
{},
54+
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
55+
{},
56+
ReleaseVirtuals
4857
> &
4958
SoftDeleteDocument
5059

@@ -55,24 +64,15 @@ export interface SemverObject {
5564
metadata?: string
5665
}
5766

58-
const ReleaseSchema = new Schema<ReleaseDoc & { semver: string | SemverObject }>(
67+
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
68+
const ReleaseSchema = new Schema<ReleaseDoc, {}, {}, ReleaseVirtuals>(
5969
{
6070
modelId: { type: String, required: true },
6171
modelCardVersion: { type: Number, required: true },
6272

6373
semver: {
6474
type: Schema.Types.Mixed,
6575
required: true,
66-
set: function (semver: string) {
67-
return semverStringToObject(semver)
68-
},
69-
get: function (semver: SemverObject | string) {
70-
if (typeof semver === 'string') {
71-
return semver
72-
} else {
73-
return semverObjectToString(semver)
74-
}
75-
},
7676
},
7777

7878
notes: { type: String, required: true },
@@ -110,6 +110,17 @@ const ReleaseSchema = new Schema<ReleaseDoc & { semver: string | SemverObject }>
110110
},
111111
)
112112

113+
ReleaseSchema.virtual('semverString').get(function () {
114+
return semverObjectToString(this.semver)
115+
})
116+
ReleaseSchema.virtual('semverString').set(function (semver: SemverObject | string) {
117+
if (typeof semver === 'string') {
118+
this.semver = semverStringToObject(semver)
119+
} else {
120+
this.semver = semver
121+
}
122+
})
123+
113124
ReleaseSchema.plugin(softDeletionPlugin)
114125
ReleaseSchema.index({ modelId: 1, semver: 1 }, { unique: true })
115126
ReleaseSchema.index({ modelId: 1 })

backend/src/routes/v2/release/getRelease.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ registerPath({
3939
})
4040

4141
interface getReleaseResponse {
42-
release: ReleaseInterface & { files: FileWithScanResultsAggregate[] }
42+
release: Omit<ReleaseInterface, 'semver'> & { semver: string; files: FileWithScanResultsAggregate[] }
4343
}
4444

4545
export const getRelease = [
@@ -52,7 +52,7 @@ export const getRelease = [
5252
const release = await getReleaseBySemver(req.user, modelId, semver)
5353
await audit.onViewRelease(req, release)
5454
const files = await getFilesByIds(req.user, modelId, release.fileIds)
55-
const releaseWithFiles = { ...release.toObject(), files, kind: ResponseKind.Comment }
55+
const releaseWithFiles = { ...release.toObject(), semver: release.semverString, files, kind: ResponseKind.Comment }
5656

5757
res.json({
5858
release: releaseWithFiles,

backend/src/seeds/data/models.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,11 @@ export const release: ReleaseInterface = {
7676
modelId: 'example',
7777
modelCardVersion: 0,
7878

79-
semver: 'v1.0.0',
79+
semver: {
80+
major: 1,
81+
minor: 0,
82+
patch: 0,
83+
},
8084
notes: `This makes major steps forward in both speed and quality of \
8185
results given by the model. Here is a table of the updated speed:
8286

backend/src/services/mirroredModel/exporters/documents.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { UserInterface } from '../../../models/User.js'
1010
import { MirrorExportLogData, MirrorKind } from '../../../types/types.js'
1111
import config from '../../../utils/config.js'
1212
import { BadReq, InternalError } from '../../../utils/error.js'
13+
import { semverObjectToString } from '../../../utils/semver.js'
1314
import { getFilesByIds, getTotalFileSize } from '../../file.js'
1415
import log from '../../log.js'
1516
import { getModelCardRevisions } from '../../model.js'
@@ -40,7 +41,7 @@ export class DocumentsExporter extends BaseExporter {
4041

4142
protected async _init() {
4243
if (this.releases.length > 0) {
43-
const semvers = this.getSemvers()
44+
const semvers = this.getSemvers().map((semver) => semverObjectToString(semver))
4445
const fileIds = await getAllFileIds(this.model.id, semvers)
4546
this.files = await getFilesByIds(this.user, this.model.id, fileIds)
4647

@@ -201,7 +202,7 @@ export class DocumentsExporter extends BaseExporter {
201202
error,
202203
modelId: this.model.id,
203204
mirroredModelId: this.model!.settings.mirror.destinationModelId!,
204-
releaseId: release.id,
205+
releaseId: release._id.toString(),
205206
...this.logData,
206207
})
207208
}

backend/src/services/mirroredModel/importers/documents.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export type MongoDocumentMirrorInformation = {
2828
metadata: DocumentsMirrorMetadata
2929
modelCardVersions: ModelCardRevisionDoc['version'][]
3030
newModelCards: Omit<ModelCardRevisionDoc, '_id'>[]
31-
releaseSemvers: ReleaseDoc['semver'][]
31+
releaseSemvers: ReleaseDoc['semverString'][]
3232
newReleases: Omit<ReleaseDoc, '_id'>[]
3333
fileIds: { key: Types.ObjectId; name: string }[]
3434
imageIds: string[]
@@ -118,7 +118,7 @@ export class DocumentsImporter extends BaseImporter {
118118
} as DistributionPackageName),
119119
)
120120
}
121-
this.releaseSemvers.push(release.semver)
121+
this.releaseSemvers.push(release.semverString)
122122
const savedRelease = await saveImportedRelease(release)
123123
if (savedRelease) {
124124
this.newReleases.push(savedRelease)

backend/src/services/model.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ export async function removeModel(user: UserInterface, modelId: string, kind?: E
231231
deleteReleases(
232232
user,
233233
modelId,
234-
allModelReleases.flatMap((release) => release.semver),
234+
allModelReleases.flatMap((release) => release.semverString),
235235
true,
236236
session,
237237
),

backend/src/services/release.ts

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { findDuplicates } from '../utils/array.js'
1616
import { toEntity } from '../utils/entity.js'
1717
import { BadReq, Forbidden, InternalError, NotFound } from '../utils/error.js'
1818
import { isMongoServerError } from '../utils/mongo.js'
19+
import { semverObjectToString, semverStringToObject } from '../utils/semver.js'
1920
import { arrayOfObjectsHasKeysOfType, hasKeysOfType } from '../utils/typeguards.js'
2021
import { getFileById, getFilesByIds } from './file.js'
2122
import log from './log.js'
@@ -49,8 +50,8 @@ export function isReleaseDoc(data: unknown): data is ReleaseDoc {
4950
}
5051

5152
export async function validateRelease(user: UserInterface, model: ModelDoc, release: ReleaseDoc) {
52-
if (!semver.valid(release.semver)) {
53-
throw BadReq(`The version '${release.semver}' is not a valid semver value.`)
53+
if (!semver.valid(release.semverString)) {
54+
throw BadReq(`The version '${release.semverString}' is not a valid semver value.`)
5455
}
5556

5657
if (release.images && release.images.length > 0) {
@@ -89,7 +90,7 @@ export async function validateRelease(user: UserInterface, model: ModelDoc, rele
8990
if (isBailoError(e) && e.code === 404) {
9091
throw BadReq('Unable to create release as the file cannot be found.', {
9192
fileId,
92-
semver: release.semver,
93+
semver: release.semverString,
9394
modelId: release.modelId,
9495
})
9596
}
@@ -125,10 +126,9 @@ export async function validateRelease(user: UserInterface, model: ModelDoc, rele
125126
}
126127

127128
export type CreateReleaseParams = Optional<
128-
Pick<
129-
ReleaseInterface,
130-
'modelId' | 'modelCardVersion' | 'semver' | 'notes' | 'minor' | 'draft' | 'fileIds' | 'images'
131-
>,
129+
Pick<ReleaseInterface, 'modelId' | 'modelCardVersion' | 'notes' | 'minor' | 'draft' | 'fileIds' | 'images'> & {
130+
semver: string
131+
},
132132
'modelCardVersion'
133133
>
134134
export async function createRelease(user: UserInterface, releaseParams: CreateReleaseParams) {
@@ -150,7 +150,7 @@ export async function createRelease(user: UserInterface, releaseParams: CreateRe
150150

151151
const deletedRelease = await ReleaseModel.findOne({
152152
modelId: releaseParams.modelId,
153-
semver: releaseParams.semver,
153+
semver: semverStringToObject(releaseParams.semver),
154154
deleted: true,
155155
})
156156
if (deletedRelease) {
@@ -162,6 +162,8 @@ export async function createRelease(user: UserInterface, releaseParams: CreateRe
162162
const release = new ReleaseModel({
163163
createdBy: user.dn,
164164
...releaseParams,
165+
// override semver string to be the object representation
166+
semver: semverStringToObject(releaseParams.semver),
165167
})
166168

167169
await validateRelease(user, model, release)
@@ -198,7 +200,7 @@ export async function createRelease(user: UserInterface, releaseParams: CreateRe
198200
sendWebhooks(
199201
release.modelId,
200202
WebhookEvent.CreateRelease,
201-
`Release ${release.semver} has been created for model ${release.modelId}`,
203+
`Release ${release.semverString} has been created for model ${release.modelId}`,
202204
{ release },
203205
)
204206

@@ -223,7 +225,10 @@ export async function updateRelease(user: UserInterface, modelId: string, semver
223225
modelId: modelId,
224226
})
225227
}
226-
const updatedRelease = await ReleaseModel.findOneAndUpdate({ modelId, semver }, { $set: release })
228+
const updatedRelease = await ReleaseModel.findOneAndUpdate(
229+
{ modelId, semver: semverStringToObject(semver) },
230+
{ $set: release },
231+
)
227232

228233
if (!updatedRelease) {
229234
throw NotFound(`The requested release was not found.`, { modelId, semver })
@@ -232,7 +237,7 @@ export async function updateRelease(user: UserInterface, modelId: string, semver
232237
sendWebhooks(
233238
release.modelId,
234239
WebhookEvent.UpdateRelease,
235-
`ReleaseModel ${release.semver} has been updated for model ${release.modelId}`,
240+
`ReleaseModel ${release.semverString} has been updated for model ${release.modelId}`,
236241
{ release },
237242
)
238243

@@ -245,7 +250,7 @@ export async function newReleaseComment(user: UserInterface, modelId: string, se
245250
throw BadReq(`Cannot create a new comment on a mirrored model.`)
246251
}
247252

248-
const release = await ReleaseModel.findOne({ modelId, semver })
253+
const release = await ReleaseModel.findOne({ modelId, semver: semverStringToObject(semver) })
249254
if (!release) {
250255
throw NotFound(`The requested release was not found.`, { modelId, semver })
251256
}
@@ -320,12 +325,13 @@ export async function getModelReleases(
320325

321326
export async function getReleasesForExport(user: UserInterface, modelId: string, semvers: string[]) {
322327
const model = await getModelById(user, modelId)
328+
const semverObjects = semvers.map((semver) => semverStringToObject(semver))
323329
const releases = await ReleaseModel.find({
324330
modelId,
325-
semver: { $in: semvers },
331+
semver: { $in: semverObjects },
326332
})
327333

328-
const missing = semvers.filter((x) => !releases.some((release) => release.semver === x))
334+
const missing = semvers.filter((x) => !releases.some((release) => release.semverString === x))
329335
if (missing.length > 0) {
330336
throw NotFound('The following releases were not found.', { modelId, releases: missing })
331337
}
@@ -335,43 +341,19 @@ export async function getReleasesForExport(user: UserInterface, modelId: string,
335341
if (failedReleases.length > 0) {
336342
throw Forbidden('You do not have the necessary permissions to export these releases.', {
337343
modelId,
338-
releases: failedReleases.map((release) => release.semver),
344+
releases: failedReleases.map((release) => release.semverString),
339345
user,
340346
})
341347
}
342348

343349
return releases
344350
}
345351

346-
export function semverStringToObject(semver: string): SemverObject {
347-
const vIdentifierIndex = semver.indexOf('v')
348-
const trimmedSemver = vIdentifierIndex === -1 ? semver : semver.slice(vIdentifierIndex + 1)
349-
const [version, metadata] = trimmedSemver.split('-')
350-
const [major, minor, patch] = version.split('.')
351-
const majorNum: number = Number(major)
352-
const minorNum: number = Number(minor)
353-
const patchNum: number = Number(patch)
354-
return { major: majorNum, minor: minorNum, patch: patchNum, ...(metadata && { metadata }) }
355-
}
356-
357-
export function semverObjectToString(semver: SemverObject): string {
358-
if (!semver) {
359-
return ''
360-
}
361-
let metadata: string
362-
if (semver.metadata != undefined) {
363-
metadata = `-${semver.metadata}`
364-
} else {
365-
metadata = ``
366-
}
367-
return `${semver.major}.${semver.minor}.${semver.patch}${metadata}`
368-
}
369-
370352
export async function getReleaseBySemver(user: UserInterface, model: string | ModelDoc, semver: string) {
371353
if (typeof model === 'string') {
372354
model = await getModelById(user, model)
373355
}
374-
const release = await ReleaseModel.findOne({ modelId: model.id, semver })
356+
const release = await ReleaseModel.findOne({ modelId: model.id, semver: semverStringToObject(semver) })
375357

376358
if (!release) {
377359
throw NotFound(`The requested release was not found.`, { modelId: model.id, semver })
@@ -538,7 +520,7 @@ export async function deleteReleases(
538520

539521
await release.delete(session)
540522
await removeReleaseReviews(modelId, semver, session)
541-
await removeResponsesByParentIds([...reviewsForRelease.map((review) => review.id), release.id], session)
523+
await removeResponsesByParentIds([...reviewsForRelease.map((review) => review.id), release._id.toString()], session)
542524
}
543525

544526
return { modelId, semvers }

0 commit comments

Comments
 (0)