Skip to content

Commit 26d9b38

Browse files
JulusianJulusian
authored
Merge pull request Sofie-Automation#1518 from nrkno/fix/upstream-ingest-bugs
fix: ingest parts not being updated when rank changes
2 parents 4cc2e33 + aee51ea commit 26d9b38

File tree

9 files changed

+147
-94
lines changed

9 files changed

+147
-94
lines changed

‎packages/job-worker/src/blueprints/ingest/MutableIngestRundownImpl.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -347,11 +347,7 @@ export class MutableIngestRundownImpl<TRundownPayload = unknown, TSegmentPayload
347347
}
348348

349349
// Regenerate the segment if there are substantial changes
350-
if (
351-
segmentInfo.segmentHasChanges ||
352-
segmentInfo.partOrderHasChanged ||
353-
segmentInfo.partIdsWithChanges.length > 0
354-
) {
350+
if (segmentInfo.segmentHasChanges || segmentInfo.partIdsWithChanges.length > 0) {
355351
segmentsToRegenerate.push(ingestSegment)
356352
}
357353
})

‎packages/job-worker/src/blueprints/ingest/MutableIngestSegmentImpl.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ export interface MutableIngestSegmentChanges {
2020
allCacheObjectIds: SofieIngestDataCacheObjId[]
2121
segmentHasChanges: boolean
2222
partIdsWithChanges: string[]
23-
partOrderHasChanged: boolean
2423
originalExternalId: string
2524
}
2625

@@ -131,11 +130,13 @@ export class MutableIngestSegmentImpl<TSegmentPayload = unknown, TPartPayload =
131130
if (beforeIndex === -1) throw new Error(`Part "${beforePartExternalId}" not found`)
132131

133132
this.#parts.splice(beforeIndex, 0, newPart)
133+
134+
this.#partOrderHasChanged = true
134135
} else {
135136
this.#parts.push(newPart)
136-
}
137137

138-
this.#partOrderHasChanged = true
138+
// If inserting at the end, the order hasn't changed
139+
}
139140

140141
return newPart
141142
}
@@ -234,14 +235,13 @@ export class MutableIngestSegmentImpl<TSegmentPayload = unknown, TPartPayload =
234235
allCacheObjectIds.push(generator.getPartObjectId(ingestPart.externalId))
235236
ingestParts.push(ingestPart)
236237

237-
if (part.checkAndClearChangesFlags()) {
238+
if (this.#partOrderHasChanged || part.checkAndClearChangesFlags()) {
238239
changedCacheObjects.push(generator.generatePartObject(segmentId, ingestPart))
239240
partIdsWithChanges.push(ingestPart.externalId)
240241
}
241242
})
242243

243244
const segmentHasChanges = this.#segmentHasChanges
244-
const partOrderHasChanged = this.#partOrderHasChanged
245245
const originalExternalId = this.#originalExternalId
246246

247247
// clear flags
@@ -255,7 +255,6 @@ export class MutableIngestSegmentImpl<TSegmentPayload = unknown, TPartPayload =
255255
allCacheObjectIds,
256256
segmentHasChanges,
257257
partIdsWithChanges,
258-
partOrderHasChanged,
259258
originalExternalId,
260259
}
261260
}

‎packages/job-worker/src/blueprints/ingest/__tests__/MutableIngestSegmentImpl.spec.ts‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { protectString } from '@sofie-automation/corelib/dist/protectedString'
55
import { getSegmentId } from '../../../ingest/lib'
66
import { MutableIngestPartImpl } from '../MutableIngestPartImpl'
77
import { IngestPart, IngestSegment, SofieIngestSegment } from '@sofie-automation/blueprints-integration'
8+
import { SofieIngestDataCacheObjPart } from '@sofie-automation/corelib/dist/dataModel/SofieIngestDataCache'
89

910
describe('MutableIngestSegmentImpl', () => {
1011
function getBasicIngestSegment(): SofieIngestSegment<any> {
@@ -70,7 +71,6 @@ describe('MutableIngestSegmentImpl', () => {
7071
allCacheObjectIds: ingestSegment.parts.map((p) => ingestObjectGenerator.getPartObjectId(p.externalId)),
7172
segmentHasChanges: false,
7273
partIdsWithChanges: [],
73-
partOrderHasChanged: false,
7474
originalExternalId: ingestSegment.externalId,
7575
}
7676
}
@@ -87,6 +87,17 @@ describe('MutableIngestSegmentImpl', () => {
8787
return mutableSegment.parts.map((p) => p.externalId)
8888
}
8989

90+
function pushAllPartsToChanges(
91+
expectedChanges: MutableIngestSegmentChanges,
92+
ingestSegment: SofieIngestSegment
93+
): void {
94+
const segmentId = getSegmentId(ingestObjectGenerator.rundownId, ingestSegment.externalId)
95+
for (const ingestPart of ingestSegment.parts) {
96+
expectedChanges.partIdsWithChanges.push(ingestPart.externalId)
97+
expectedChanges.changedCacheObjects.push(ingestObjectGenerator.generatePartObject(segmentId, ingestPart))
98+
}
99+
}
100+
90101
test('create basic', () => {
91102
const ingestSegment = getBasicIngestSegment()
92103
const mutableSegment = new MutableIngestSegmentImpl(clone(ingestSegment))
@@ -114,11 +125,7 @@ describe('MutableIngestSegmentImpl', () => {
114125
// check it has no changes
115126
const expectedChanges = createNoChangesObject(ingestSegment)
116127
expectedChanges.segmentHasChanges = true
117-
const segmentId = getSegmentId(ingestObjectGenerator.rundownId, ingestSegment.externalId)
118-
for (const ingestPart of ingestSegment.parts) {
119-
expectedChanges.partIdsWithChanges.push(ingestPart.externalId)
120-
expectedChanges.changedCacheObjects.push(ingestObjectGenerator.generatePartObject(segmentId, ingestPart))
121-
}
128+
pushAllPartsToChanges(expectedChanges, ingestSegment)
122129
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
123130

124131
// check changes have been cleared
@@ -279,7 +286,7 @@ describe('MutableIngestSegmentImpl', () => {
279286
const expectedIngestSegment = clone(ingestSegment)
280287
removePartFromIngestSegment(expectedIngestSegment, 'part1')
281288
const expectedChanges = createNoChangesObject(expectedIngestSegment)
282-
expectedChanges.partOrderHasChanged = true
289+
pushAllPartsToChanges(expectedChanges, expectedIngestSegment)
283290
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
284291

285292
// try removing a second time
@@ -347,14 +354,12 @@ describe('MutableIngestSegmentImpl', () => {
347354
expectedIngestSegment.parts.push({ ...newPart, rank: 3, userEditStates: {} })
348355

349356
const expectedChanges = createNoChangesObject(expectedIngestSegment)
350-
expectedChanges.partOrderHasChanged = true
351-
expectedChanges.partIdsWithChanges.push('part1')
352-
expectedChanges.changedCacheObjects.push(
353-
ingestObjectGenerator.generatePartObject(
354-
getSegmentId(ingestObjectGenerator.rundownId, ingestSegment.externalId),
355-
{ ...newPart, rank: 3, userEditStates: {} }
356-
)
357+
pushAllPartsToChanges(expectedChanges, expectedIngestSegment)
358+
const generatedCacheObject = expectedChanges.changedCacheObjects.find(
359+
(o): o is SofieIngestDataCacheObjPart => o.type === 'part' && o.data.externalId === 'part1'
357360
)
361+
expect(generatedCacheObject).toBeDefined()
362+
expect(generatedCacheObject?.data.rank).toBe(3)
358363

359364
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
360365
})
@@ -386,7 +391,6 @@ describe('MutableIngestSegmentImpl', () => {
386391
expectedIngestSegment.parts.push({ ...newPart, rank: 4, userEditStates: {} })
387392

388393
const expectedChanges = createNoChangesObject(expectedIngestSegment)
389-
expectedChanges.partOrderHasChanged = true
390394
expectedChanges.partIdsWithChanges.push('partX')
391395
expectedChanges.changedCacheObjects.push(
392396
ingestObjectGenerator.generatePartObject(
@@ -425,8 +429,14 @@ describe('MutableIngestSegmentImpl', () => {
425429
expect(mutableSegment.replacePart(newPart, 'part2')).toBeDefined()
426430
expect(getPartIdOrder(mutableSegment)).toEqual(['part0', 'part1', 'partX', 'part2', 'part3'])
427431

428-
// Only the one should have changes
429-
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator).partIdsWithChanges).toEqual(['partX'])
432+
// All should report changes because of their rank property
433+
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator).partIdsWithChanges).toEqual([
434+
'part0',
435+
'part1',
436+
'partX',
437+
'part2',
438+
'part3',
439+
])
430440

431441
// Try inserting before itself
432442
expect(() => mutableSegment.replacePart(newPart, newPart.externalId)).toThrow(
@@ -470,7 +480,7 @@ describe('MutableIngestSegmentImpl', () => {
470480

471481
// Only the one should have changes
472482
const expectedChanges = createNoChangesObject(ingestSegment)
473-
expectedChanges.partOrderHasChanged = true
483+
pushAllPartsToChanges(expectedChanges, ingestSegment)
474484
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
475485

476486
// Try inserting before itself
@@ -511,9 +521,9 @@ describe('MutableIngestSegmentImpl', () => {
511521
mutableSegment.movePartAfter('part1', 'part0')
512522
expect(getPartIdOrder(mutableSegment)).toEqual(['part0', 'part1', 'part2', 'part3'])
513523

514-
// Only the one should have changes
524+
// All the parts should be reported as having changed
515525
const expectedChanges = createNoChangesObject(ingestSegment)
516-
expectedChanges.partOrderHasChanged = true
526+
pushAllPartsToChanges(expectedChanges, ingestSegment)
517527
expect(mutableSegment.intoChangesInfo(ingestObjectGenerator)).toEqual(expectedChanges)
518528

519529
// Try inserting before itself

‎packages/job-worker/src/ingest/commit.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,8 @@ export async function updatePlayoutAfterChangingRundownInPlaylist(
576576
context: JobContext,
577577
playlist: DBRundownPlaylist,
578578
playlistLock: PlaylistLock,
579-
insertedRundown: ReadonlyDeep<DBRundown> | null
579+
insertedRundown: ReadonlyDeep<DBRundown> | null,
580+
rundownIdToForget: RundownId | null
580581
): Promise<void> {
581582
// ensure the 'old' playout is updated to remove any references to the rundown
582583
await runWithPlayoutModel(context, playlist, playlistLock, null, async (playoutModel) => {
@@ -591,6 +592,16 @@ export async function updatePlayoutAfterChangingRundownInPlaylist(
591592
return
592593
}
593594

595+
// Ensure previousPartInstance doesn't reference the old Rundown
596+
// The current and next partInstances are enforced by allowedToMoveRundownOutOfPlaylist
597+
if (
598+
rundownIdToForget &&
599+
playoutModel.previousPartInstance &&
600+
playoutModel.previousPartInstance.partInstance.rundownId === rundownIdToForget
601+
) {
602+
playoutModel.clearPreviousPartInstance()
603+
}
604+
594605
// Ensure playout is in sync
595606

596607
if (insertedRundown) {
@@ -711,7 +722,7 @@ export async function removeRundownFromPlaylistAndUpdatePlaylist(
711722

712723
if (updatedPlaylist) {
713724
// ensure the 'old' playout is updated to remove any references to the rundown
714-
await updatePlayoutAfterChangingRundownInPlaylist(context, updatedPlaylist, playlistLock, null)
725+
await updatePlayoutAfterChangingRundownInPlaylist(context, updatedPlaylist, playlistLock, null, rundownId)
715726
}
716727
}
717728

‎packages/job-worker/src/ingest/runOperation.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ export async function runIngestUpdateOperationBase(
202202
}
203203
} finally {
204204
// Ensure we save the nrcs ingest data
205-
// await pSaveNrcsIngestChanges
205+
await pSaveNrcsIngestChanges
206206

207207
span?.end()
208208
}

‎packages/job-worker/src/ingest/syncChangesToPartInstance.ts‎

Lines changed: 74 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -267,70 +267,86 @@ export function findInstancesToSync(
267267

268268
const instancesToSync: PartInstanceToSync[] = []
269269
if (currentPartInstance) {
270-
// If the currentPartInstance is adlibbed we probably also need to find the earliest
271-
// non-adlibbed Part within this segment and check it for updates too. It may have something
272-
// changed (like timing) that will affect what's going on.
273-
// The previous "planned" Part Instance needs to be inserted into the `instances` first, so that
274-
// it's ran first through the blueprints.
275-
if (currentPartInstance.partInstance.orphaned === 'adlib-part') {
276-
const partAndPartInstance = findLastUnorphanedPartInstanceInSegment(
277-
playoutModel,
278-
currentPartInstance.partInstance
279-
)
280-
if (partAndPartInstance) {
281-
const lastPartRundownModel = findPlayoutRundownModel(
282-
playoutRundownModel,
283-
playoutModel,
284-
partAndPartInstance.partInstance.partInstance.part.rundownId
285-
)
286-
287-
insertToSyncedInstanceCandidates(
288-
context,
289-
instancesToSync,
270+
try {
271+
// If the currentPartInstance is adlibbed we probably also need to find the earliest
272+
// non-adlibbed Part within this segment and check it for updates too. It may have something
273+
// changed (like timing) that will affect what's going on.
274+
// The previous "planned" Part Instance needs to be inserted into the `instances` first, so that
275+
// it's ran first through the blueprints.
276+
if (currentPartInstance.partInstance.orphaned === 'adlib-part') {
277+
const partAndPartInstance = findLastUnorphanedPartInstanceInSegment(
290278
playoutModel,
291-
lastPartRundownModel,
292-
ingestModel,
293-
partAndPartInstance.partInstance,
294-
null,
295-
partAndPartInstance.part,
296-
'previous'
279+
currentPartInstance.partInstance
297280
)
281+
if (partAndPartInstance) {
282+
try {
283+
const lastPartRundownModel = findPlayoutRundownModel(
284+
playoutRundownModel,
285+
playoutModel,
286+
partAndPartInstance.partInstance.partInstance.part.rundownId
287+
)
288+
289+
insertToSyncedInstanceCandidates(
290+
context,
291+
instancesToSync,
292+
playoutModel,
293+
lastPartRundownModel,
294+
ingestModel,
295+
partAndPartInstance.partInstance,
296+
null,
297+
partAndPartInstance.part,
298+
'previous'
299+
)
300+
} catch (err) {
301+
logger.error(
302+
`Failed to prepare previousPartInstance for syncChangesToPartInstances: ${stringifyError(
303+
err
304+
)}`
305+
)
306+
}
307+
}
298308
}
299-
}
300309

301-
// We can now run the current Part Instance.
302-
const currentPartRundownModel = findPlayoutRundownModel(
303-
playoutRundownModel,
304-
playoutModel,
305-
currentPartInstance.partInstance.part.rundownId
306-
)
307-
findPartAndInsertToSyncedInstanceCandidates(
308-
context,
309-
instancesToSync,
310-
playoutModel,
311-
currentPartRundownModel,
312-
ingestModel,
313-
currentPartInstance,
314-
previousPartInstance,
315-
'current'
316-
)
310+
// We can now run the current Part Instance.
311+
const currentPartRundownModel = findPlayoutRundownModel(
312+
playoutRundownModel,
313+
playoutModel,
314+
currentPartInstance.partInstance.part.rundownId
315+
)
316+
findPartAndInsertToSyncedInstanceCandidates(
317+
context,
318+
instancesToSync,
319+
playoutModel,
320+
currentPartRundownModel,
321+
ingestModel,
322+
currentPartInstance,
323+
previousPartInstance,
324+
'current'
325+
)
326+
} catch (err) {
327+
logger.error(`Failed to prepare currentPartInstance for syncChangesToPartInstances: ${stringifyError(err)}`)
328+
}
317329
}
318330
if (nextPartInstance) {
319-
const nextPartRundownModel = findPlayoutRundownModel(
320-
playoutRundownModel,
321-
playoutModel,
322-
nextPartInstance.partInstance.part.rundownId
323-
)
324-
findPartAndInsertToSyncedInstanceCandidates(
325-
context,
326-
instancesToSync,
327-
playoutModel,
328-
nextPartRundownModel,
329-
ingestModel,
330-
nextPartInstance,
331-
currentPartInstance,
332-
currentPartInstance?.isTooCloseToAutonext(false) ? 'current' : 'next'
333-
)
331+
try {
332+
const nextPartRundownModel = findPlayoutRundownModel(
333+
playoutRundownModel,
334+
playoutModel,
335+
nextPartInstance.partInstance.part.rundownId
336+
)
337+
findPartAndInsertToSyncedInstanceCandidates(
338+
context,
339+
instancesToSync,
340+
playoutModel,
341+
nextPartRundownModel,
342+
ingestModel,
343+
nextPartInstance,
344+
currentPartInstance,
345+
currentPartInstance?.isTooCloseToAutonext(false) ? 'current' : 'next'
346+
)
347+
} catch (err) {
348+
logger.error(`Failed to prepare nextPartInstance for syncChangesToPartInstances: ${stringifyError(err)}`)
349+
}
334350
}
335351

336352
return instancesToSync

‎packages/job-worker/src/playout/model/PlayoutModel.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,12 @@ export interface PlayoutModel extends PlayoutModelReadonly, StudioPlayoutModelBa
225225
*/
226226
clearSelectedPartInstances(): void
227227

228+
/**
229+
* Clear the currently selected previousPartInstance.
230+
* This can be useful if it references a Rundown that has been removed from the Playlist
231+
*/
232+
clearPreviousPartInstance(): void
233+
228234
/**
229235
* Insert an adlibbed PartInstance into the RundownPlaylist
230236
* @param part Part to insert

0 commit comments

Comments
 (0)