diff --git a/libervia/backend/memory/sqla.py b/libervia/backend/memory/sqla.py index ba609f5a..cb123b4b 100644 --- a/libervia/backend/memory/sqla.py +++ b/libervia/backend/memory/sqla.py @@ -1112,12 +1112,12 @@ class Storage: async def file_update( self, file_id: str, column: str, update_cb: Callable[[dict], None] ) -> None: - """Update a column value using a method to avoid race conditions + """Update a column value using a method to avoid race conditions. - the older value will be retrieved from database, then update_cb will be applied to + The older value will be retrieved from database, then update_cb will be applied to update it, and file will be updated checking that older value has not been changed meanwhile by another user. If it has changed, it tries again a couple of times - before failing + before failing. @param column: column name (only "access" or "extra" are allowed) @param update_cb: method to update the value of the colum the method will take older value as argument, and must update it in place @@ -1127,43 +1127,52 @@ class Storage: """ if column not in ("access", "extra"): raise exceptions.InternalError("bad column name") - orm_col = getattr(File, column) for i in range(5): async with self.session() as session: try: - value = ( - await session.execute(select(orm_col).filter_by(id=file_id)) - ).scalar_one() + # We use raw SQL to bypass ORM processing and get the literal database + # value, which will be used for comparison on update: if the value has + # been changed between the select and update, the update will fail, + # preventing race condition. + stmt = text(f"SELECT {column} FROM files WHERE id = :file_id") + result = await session.execute(stmt, {"file_id": file_id}) + row = result.first() + if not row: + raise NoResultFound() + + raw_old_value = row[0] + + # We deserialise for the callback. + new_value = {} if raw_old_value is None else json.loads(raw_old_value) + update_cb(new_value) + + where_clause = text(f"id = :file_id AND {column} = :raw_old_value") + params = {"file_id": file_id, "raw_old_value": raw_old_value} + + update_stmt = text( + f"UPDATE files SET {column} = :new_value WHERE " + + where_clause.text + ) + params["new_value"] = json.dumps(new_value, ensure_ascii=False) + + result = await session.execute(update_stmt, params) + await session.commit() + except NoResultFound: raise exceptions.NotFound - old_value = copy.deepcopy(value) - update_cb(value) - stmt = update(File).filter_by(id=file_id).values({column: value}) - if not old_value: - # because JsonDefaultDict convert NULL to an empty dict, we have to - # test both for empty dict and None when we have an empty dict - stmt = stmt.where((orm_col == None) | (orm_col == old_value)) - else: - stmt = stmt.where(orm_col == old_value) - result = await session.execute(stmt) - await session.commit() - if result.rowcount == 1: - break + if result.rowcount == 1: + break - log.warning( - _( + log.warning( "table not updated, probably due to race condition, trying again " - "({tries})" - ).format(tries=i + 1) - ) + f"({i+1})." + ) else: raise exceptions.DatabaseError( - _("Can't update file {file_id} due to race condition").format( - file_id=file_id - ) + f"Can't update file {file_id} due to race condition" ) @aio