Fix job meta requests

changelog.d/20240306_181839_kirill.9992_fix_meta_requests.md

@@ -0,0 +1,6 @@
+### Fixed
+
+- Sending `PATCH /jobs/{id}/data/meta` on each job save even if nothing changed in meta data
+  (<https://github.com/opencv/cvat/pull/7560>)
+- Sending `GET /jobs/{id}/data/meta` twice on each job load
+  (<https://github.com/opencv/cvat/pull/7560>)

cvat-core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "cvat-core",
-  "version": "15.0.0",
+  "version": "15.0.1",
   "type": "module",
   "description": "Part of Computer Vision Tool which presents an interface for client-side integration",
   "main": "src/api.ts",

cvat-core/src/common.ts

@@ -95,6 +95,14 @@ export function checkObjectType(name, value, type, instance?): boolean {
 export class FieldUpdateTrigger {
     #updatedFlags: Record<string, boolean> = {};
 
+    get(key: string): boolean {
+        return this.#updatedFlags[key] || false;
+    }
+
+    resetField(key: string): void {
+        delete this.#updatedFlags[key];
+    }
+
     reset(): void {
         this.#updatedFlags = {};
     }

cvat-core/src/frames.ts

@@ -11,10 +11,11 @@ import PluginRegistry from './plugins';
 import serverProxy from './server-proxy';
 import { SerializedFramesMetaData } from './server-response-types';
 import { Exception, ArgumentError, DataError } from './exceptions';
+import { FieldUpdateTrigger } from './common';
 
 // frame storage by job id
 const frameDataCache: Record<string, {
-    meta: Omit<SerializedFramesMetaData, 'deleted_frames'> & { deleted_frames: Record<number, boolean> };
+    meta: FramesMetaData;
     chunkSize: number;
     mode: 'annotation' | 'interpolation';
     startFrame: number;
@@ -36,9 +37,12 @@ const frameDataCache: Record<string, {
     getChunk: (chunkNumber: number, quality: ChunkQuality) => Promise<ArrayBuffer>;
 }> = {};
 
+// frame meta data storage by job id
+const frameMetaCache: Record<string, Promise<FramesMetaData>> = {};
+
 export class FramesMetaData {
     public chunkSize: number;
-    public deletedFrames: number[];
+    public deletedFrames: Record<number, boolean>;
     public includedFrames: number[];
     public frameFilter: string;
     public frames: {
@@ -52,10 +56,12 @@ export class FramesMetaData {
     public startFrame: number;
     public stopFrame: number;
 
-    constructor(initialData: SerializedFramesMetaData) {
-        const data: SerializedFramesMetaData = {
+    #updateTrigger: FieldUpdateTrigger;
+
+    constructor(initialData: Omit<SerializedFramesMetaData, 'deleted_frames'> & { deleted_frames: Record<number, boolean> }) {
+        const data: typeof initialData = {
             chunk_size: undefined,
-            deleted_frames: [],
+            deleted_frames: {},
             included_frames: [],
             frame_filter: undefined,
             frames: [],
@@ -65,9 +71,35 @@ export class FramesMetaData {
             stop_frame: undefined,
         };
 
+        this.#updateTrigger = new FieldUpdateTrigger();
+
         for (const property in data) {
             if (Object.prototype.hasOwnProperty.call(data, property) && property in initialData) {
-                data[property] = initialData[property];
+                if (property === 'deleted_frames') {
+                    const update = (frame: string, remove: boolean): void => {
+                        if (this.#updateTrigger.get(`deletedFrames:${frame}:${!remove}`)) {
+                            this.#updateTrigger.resetField(`deletedFrames:${frame}:${!remove}`);
+                        } else {
+                            this.#updateTrigger.update(`deletedFrames:${frame}:${remove}`);
+                        }
+                    };
+
+                    const handler = {
+                        set: (target, prop, value) => {
+                            update(prop, value);
+                            return Reflect.set(target, prop, value);
+                        },
+                        deleteProperty: (target, prop) => {
+                            if (prop in target) {
+                                update(prop, false);
+                            }
+                            return Reflect.deleteProperty(target, prop);
+                        },
+                    };
+                    data[property] = new Proxy(initialData[property], handler);
+                } else {
+                    data[property] = initialData[property];
+                }
             }
         }
 
@@ -104,6 +136,14 @@ export class FramesMetaData {
             }),
         );
     }
+
+    getUpdated(): Record<string, unknown> {
+        return this.#updateTrigger.getUpdated(this);
+    }
+
+    resetUpdated(): void {
+        this.#updateTrigger.reset();
+    }
 }
 
 export class FrameData {
@@ -379,14 +419,44 @@ Object.defineProperty(FrameData.prototype.data, 'implementation', {
     writable: false,
 });
 
+async function getJobMeta(jobID: number): Promise<FramesMetaData> {
+    if (!frameMetaCache[jobID]) {
+        frameMetaCache[jobID] = serverProxy.frames.getMeta('job', jobID)
+            .then((serverMeta) => new FramesMetaData({
+                ...serverMeta,
+                deleted_frames: Object.fromEntries(serverMeta.deleted_frames.map((_frame) => [_frame, true])),
+            }))
+            .catch((error) => {
+                delete frameMetaCache[jobID];
+                throw error;
+            });
+    }
+    return frameMetaCache[jobID];
+}
+
+async function saveJobMeta(meta: FramesMetaData, jobID: number): Promise<FramesMetaData> {
+    frameMetaCache[jobID] = serverProxy.frames.saveMeta('job', jobID, {
+        deleted_frames: Object.keys(meta.deletedFrames).map((frame) => +frame),
+    })
+        .then((serverMeta) => new FramesMetaData({
+            ...serverMeta,
+            deleted_frames: Object.fromEntries(serverMeta.deleted_frames.map((_frame) => [_frame, true])),
+        }))
+        .catch((error) => {
+            delete frameMetaCache[jobID];
+            throw error;
+        });
+    return frameMetaCache[jobID];
+}
+
 function getFrameMeta(jobID, frame): SerializedFramesMetaData['frames'][0] {
     const { meta, mode, startFrame } = frameDataCache[jobID];
     let frameMeta = null;
     if (mode === 'interpolation' && meta.frames.length === 1) {
         // video tasks have 1 frame info, but image tasks will have many infos
         [frameMeta] = meta.frames;
     } else if (mode === 'annotation' || (mode === 'interpolation' && meta.frames.length > 1)) {
-        if (frame > meta.stop_frame) {
+        if (frame > meta.stopFrame) {
             throw new ArgumentError(`Meta information about frame ${frame} can't be received from the server`);
         }
         frameMeta = meta.frames[frame - startFrame];
@@ -502,23 +572,20 @@ export async function getFrame(
 ): Promise<FrameData> {
     if (!(jobID in frameDataCache)) {
         const blockType = chunkType === 'video' ? BlockType.MP4VIDEO : BlockType.ARCHIVE;
-        const meta = await serverProxy.frames.getMeta('job', jobID);
-        const updatedMeta = {
-            ...meta,
-            deleted_frames: Object.fromEntries(meta.deleted_frames.map((_frame) => [_frame, true])),
-        };
-        const mean = updatedMeta.frames.reduce((a, b) => a + b.width * b.height, 0) / updatedMeta.frames.length;
+        const meta = await getJobMeta(jobID);
+
+        const mean = meta.frames.reduce((a, b) => a + b.width * b.height, 0) / meta.frames.length;
         const stdDev = Math.sqrt(
-            updatedMeta.frames.map((x) => (x.width * x.height - mean) ** 2).reduce((a, b) => a + b) /
-            updatedMeta.frames.length,
+            meta.frames.map((x) => (x.width * x.height - mean) ** 2).reduce((a, b) => a + b) /
+            meta.frames.length,
         );
 
         // limit of decoded frames cache by 2GB
         const decodedBlocksCacheSize = Math.min(
             Math.floor((2048 * 1024 * 1024) / ((mean + stdDev) * 4 * chunkSize)) || 1, 10,
         );
         frameDataCache[jobID] = {
-            meta: updatedMeta,
+            meta,
             chunkSize,
             mode,
             startFrame,
@@ -553,15 +620,15 @@ export async function getFrame(
         name: frameMeta.name,
         related_files: frameMeta.related_files,
         frameNumber: frame,
-        deleted: frame in frameDataCache[jobID].meta.deleted_frames,
+        deleted: frame in frameDataCache[jobID].meta.deletedFrames,
         jobID,
     });
 }
 
 export async function getDeletedFrames(instanceType: 'job' | 'task', id): Promise<Record<number, boolean>> {
     if (instanceType === 'job') {
         const { meta } = frameDataCache[id];
-        return meta.deleted_frames;
+        return meta.deletedFrames;
     }
 
     if (instanceType === 'task') {
@@ -574,65 +641,42 @@ export async function getDeletedFrames(instanceType: 'job' | 'task', id): Promis
 
 export function deleteFrame(jobID: number, frame: number): void {
     const { meta } = frameDataCache[jobID];
-    meta.deleted_frames[frame] = true;
+    meta.deletedFrames[frame] = true;
 }
 
 export function restoreFrame(jobID: number, frame: number): void {
     const { meta } = frameDataCache[jobID];
-    if (frame in meta.deleted_frames) {
-        delete meta.deleted_frames[frame];
-    }
+    delete meta.deletedFrames[frame];
 }
 
 export async function patchMeta(jobID: number): Promise<void> {
     const { meta } = frameDataCache[jobID];
-    const newMeta = await serverProxy.frames.saveMeta('job', jobID, {
-        deleted_frames: Object.keys(meta.deleted_frames).map((frame) => +frame),
-    });
-    const prevDeletedFrames = meta.deleted_frames;
+    const updatedFields = meta.getUpdated();
 
-    // it is important do not overwrite the object, it is why we working on keys in two loops below
-    for (const frame of Object.keys(prevDeletedFrames)) {
-        delete prevDeletedFrames[frame];
+    if (Object.keys(updatedFields).length) {
+        const newMeta = await saveJobMeta(meta, jobID);
+        frameDataCache[jobID].meta = newMeta;
     }
-    for (const frame of newMeta.deleted_frames) {
-        prevDeletedFrames[frame] = true;
-    }
-
-    frameDataCache[jobID].meta = {
-        ...newMeta,
-        deleted_frames: prevDeletedFrames,
-    };
-    frameDataCache[jobID].meta.deleted_frames = prevDeletedFrames;
 }
 
 export async function findFrame(
     jobID: number, frameFrom: number, frameTo: number, filters: { offset?: number, notDeleted: boolean },
 ): Promise<number | null> {
     const offset = filters.offset || 1;
-    let meta;
-    if (!frameDataCache[jobID]) {
-        const serverMeta = await serverProxy.frames.getMeta('job', jobID);
-        meta = {
-            ...serverMeta,
-            deleted_frames: Object.fromEntries(serverMeta.deleted_frames.map((_frame) => [_frame, true])),
-        };
-    } else {
-        meta = frameDataCache[jobID].meta;
-    }
+    const meta = await getJobMeta(jobID);
 
     const sign = Math.sign(frameTo - frameFrom);
     const predicate = sign > 0 ? (frame) => frame <= frameTo : (frame) => frame >= frameTo;
     const update = sign > 0 ? (frame) => frame + 1 : (frame) => frame - 1;
     let framesCounter = 0;
     let lastUndeletedFrame = null;
     const check = (frame): boolean => {
-        if (meta.included_frames) {
-            return (meta.included_frames.includes(frame)) &&
-            (!filters.notDeleted || !(frame in meta.deleted_frames));
+        if (meta.includedFrames) {
+            return (meta.includedFrames.includes(frame)) &&
+            (!filters.notDeleted || !(frame in meta.deletedFrames));
         }
         if (filters.notDeleted) {
-            return !(frame in meta.deleted_frames);
+            return !(frame in meta.deletedFrames);
         }
         return true;
     };
@@ -660,5 +704,6 @@ export function getCachedChunks(jobID): number[] {
 export function clear(jobID: number): void {
     if (jobID in frameDataCache) {
         delete frameDataCache[jobID];
+        delete frameMetaCache[jobID];
     }
 }

tests/cypress/e2e/features/masks_basics.js

@@ -163,6 +163,7 @@ context('Manipulations with masks', { scrollBehavior: false }, () => {
     describe('Tests to make sure that empty masks cannot be created', () => {
         beforeEach(() => {
             cy.removeAnnotations();
+            cy.saveJob('PUT');
         });
 
         function checkEraseTools(baseTool = '.cvat-brush-tools-brush', disabled = true) {

tests/cypress/e2e/features/single_object_annotation.js

@@ -149,7 +149,7 @@ context('Single object annotation mode', { scrollBehavior: false }, () => {
     describe('Tests basic features of single shape annotation mode', () => {
         afterEach(() => {
             cy.removeAnnotations();
-            cy.saveJob();
+            cy.saveJob('PUT');
         });
 
         it('Check basic single shape annotation pipeline for polygon', () => {