From 4565774669bc3c1b11cc726d577946953dbe53c5 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 14 Feb 2023 08:59:27 +0100 Subject: [PATCH] Fix retrying update on sql serialization conflict --- server/controllers/api/video-channel.ts | 6 ++---- server/controllers/api/video-playlist.ts | 6 ++---- server/controllers/api/videos/update.ts | 6 ++---- server/helpers/database-utils.ts | 6 ++---- server/lib/activitypub/actors/updater.ts | 15 ++++----------- server/lib/activitypub/videos/updater.ts | 8 ++------ 6 files changed, 14 insertions(+), 33 deletions(-) diff --git a/server/controllers/api/video-channel.ts b/server/controllers/api/video-channel.ts index d7c92952a..5b9fb794a 100644 --- a/server/controllers/api/video-channel.ts +++ b/server/controllers/api/video-channel.ts @@ -273,7 +273,6 @@ async function addVideoChannel (req: express.Request, res: express.Response) { async function updateVideoChannel (req: express.Request, res: express.Response) { const videoChannelInstance = res.locals.videoChannel - const videoChannelFieldsSave = videoChannelInstance.toJSON() const oldVideoChannelAuditKeys = new VideoChannelAuditView(videoChannelInstance.toFormattedJSON()) const videoChannelInfoToUpdate = req.body as VideoChannelUpdate let doBulkVideoUpdate = false @@ -309,10 +308,9 @@ async function updateVideoChannel (req: express.Request, res: express.Response) } catch (err) { logger.debug('Cannot update the video channel.', { err }) - // Force fields we want to update // If the transaction is retried, sequelize will think the object has not changed - // So it will skip the SQL request, even if the last one was ROLLBACKed! - resetSequelizeInstance(videoChannelInstance, videoChannelFieldsSave) + // So we need to restore the previous fields + resetSequelizeInstance(videoChannelInstance) throw err } diff --git a/server/controllers/api/video-playlist.ts b/server/controllers/api/video-playlist.ts index 947f7ca77..bcb60e265 100644 --- a/server/controllers/api/video-playlist.ts +++ b/server/controllers/api/video-playlist.ts @@ -210,7 +210,6 @@ async function addVideoPlaylist (req: express.Request, res: express.Response) { async function updateVideoPlaylist (req: express.Request, res: express.Response) { const videoPlaylistInstance = res.locals.videoPlaylistFull - const videoPlaylistFieldsSave = videoPlaylistInstance.toJSON() const videoPlaylistInfoToUpdate = req.body as VideoPlaylistUpdate const wasPrivatePlaylist = videoPlaylistInstance.privacy === VideoPlaylistPrivacy.PRIVATE @@ -275,10 +274,9 @@ async function updateVideoPlaylist (req: express.Request, res: express.Response) } catch (err) { logger.debug('Cannot update the video playlist.', { err }) - // Force fields we want to update // If the transaction is retried, sequelize will think the object has not changed - // So it will skip the SQL request, even if the last one was ROLLBACKed! - resetSequelizeInstance(videoPlaylistInstance, videoPlaylistFieldsSave) + // So we need to restore the previous fields + resetSequelizeInstance(videoPlaylistInstance) throw err } diff --git a/server/controllers/api/videos/update.ts b/server/controllers/api/videos/update.ts index 260dee2b9..e6197c4b1 100644 --- a/server/controllers/api/videos/update.ts +++ b/server/controllers/api/videos/update.ts @@ -45,7 +45,6 @@ export { async function updateVideo (req: express.Request, res: express.Response) { const videoFromReq = res.locals.videoAll - const videoFieldsSave = videoFromReq.toJSON() const oldVideoAuditView = new VideoAuditView(videoFromReq.toFormattedDetailsJSON()) const videoInfoToUpdate: VideoUpdate = req.body @@ -151,10 +150,9 @@ async function updateVideo (req: express.Request, res: express.Response) { isNewVideo }) } catch (err) { - // Force fields we want to update // If the transaction is retried, sequelize will think the object has not changed - // So it will skip the SQL request, even if the last one was ROLLBACKed! - resetSequelizeInstance(videoFromReq, videoFieldsSave) + // So we need to restore the previous fields + resetSequelizeInstance(videoFromReq) throw err } finally { diff --git a/server/helpers/database-utils.ts b/server/helpers/database-utils.ts index 627381086..0e6b35503 100644 --- a/server/helpers/database-utils.ts +++ b/server/helpers/database-utils.ts @@ -78,10 +78,8 @@ function updateInstanceWithAnother > (instanc } } -function resetSequelizeInstance (instance: Model, savedFields: object) { - Object.keys(savedFields).forEach(key => { - instance[key] = savedFields[key] - }) +function resetSequelizeInstance (instance: Model) { + instance.set(instance.previous()) } function filterNonExistingModels ( diff --git a/server/lib/activitypub/actors/updater.ts b/server/lib/activitypub/actors/updater.ts index fe94af9f1..137980460 100644 --- a/server/lib/activitypub/actors/updater.ts +++ b/server/lib/activitypub/actors/updater.ts @@ -13,19 +13,12 @@ export class APActorUpdater { private accountOrChannel: MAccount | MChannel - private readonly actorFieldsSave: object - private readonly accountOrChannelFieldsSave: object - constructor ( private readonly actorObject: ActivityPubActor, private readonly actor: MActorFull ) { - this.actorFieldsSave = this.actor.toJSON() - if (this.actorObject.type === 'Group') this.accountOrChannel = this.actor.VideoChannel else this.accountOrChannel = this.actor.Account - - this.accountOrChannelFieldsSave = this.accountOrChannel.toJSON() } async update () { @@ -58,12 +51,12 @@ export class APActorUpdater { logger.info('Remote account %s updated', this.actorObject.url) } catch (err) { - if (this.actor !== undefined && this.actorFieldsSave !== undefined) { - resetSequelizeInstance(this.actor, this.actorFieldsSave) + if (this.actor !== undefined) { + resetSequelizeInstance(this.actor) } - if (this.accountOrChannel !== undefined && this.accountOrChannelFieldsSave !== undefined) { - resetSequelizeInstance(this.accountOrChannel, this.accountOrChannelFieldsSave) + if (this.accountOrChannel !== undefined) { + resetSequelizeInstance(this.accountOrChannel) } // This is just a debug because we will retry the insert diff --git a/server/lib/activitypub/videos/updater.ts b/server/lib/activitypub/videos/updater.ts index 32cbf7e07..0bf32f440 100644 --- a/server/lib/activitypub/videos/updater.ts +++ b/server/lib/activitypub/videos/updater.ts @@ -13,8 +13,6 @@ export class APVideoUpdater extends APVideoAbstractBuilder { private readonly wasPrivateVideo: boolean private readonly wasUnlistedVideo: boolean - private readonly videoFieldsSave: any - private readonly oldVideoChannel: MChannelAccountLight protected lTags: LoggerTagsFn @@ -30,8 +28,6 @@ export class APVideoUpdater extends APVideoAbstractBuilder { this.oldVideoChannel = this.video.VideoChannel - this.videoFieldsSave = this.video.toJSON() - this.lTags = loggerTagsFactory('ap', 'video', 'update', video.uuid, video.url) } @@ -156,8 +152,8 @@ export class APVideoUpdater extends APVideoAbstractBuilder { } private catchUpdateError (err: Error) { - if (this.video !== undefined && this.videoFieldsSave !== undefined) { - resetSequelizeInstance(this.video, this.videoFieldsSave) + if (this.video !== undefined) { + resetSequelizeInstance(this.video) } // This is just a debug because we will retry the insert