diff --git a/cvat-core/src/annotations-saver.js b/cvat-core/src/annotations-saver.js index 3e662f9a..2a27e2d5 100644 --- a/cvat-core/src/annotations-saver.js +++ b/cvat-core/src/annotations-saver.js @@ -18,25 +18,35 @@ this.id = session.id; this.version = version; this.collection = collection; - this.initialObjects = []; + this.initialObjects = {}; this.hash = this._getHash(); // We need use data from export instead of initialData // Otherwise we have differ keys order and JSON comparison code incorrect const exported = this.collection.export(); + + this._resetState(); for (const shape of exported.shapes) { - this.initialObjects[shape.id] = shape; + this.initialObjects.shapes[shape.id] = shape; } for (const track of exported.tracks) { - this.initialObjects[track.id] = track; + this.initialObjects.tracks[track.id] = track; } for (const tag of exported.tags) { - this.initialObjects[tag.id] = tag; + this.initialObjects.tags[tag.id] = tag; } } + _resetState() { + this.initialObjects = { + shapes: {}, + tracks: {}, + tags: {}, + }; + } + _getHash() { const exported = this.collection.export(); return JSON.stringify(exported); @@ -95,9 +105,9 @@ // Find created and updated objects for (const type of Object.keys(exported)) { for (const object of exported[type]) { - if (object.id in this.initialObjects) { + if (object.id in this.initialObjects[type]) { const exportedHash = JSON.stringify(object); - const initialHash = JSON.stringify(this.initialObjects[object.id]); + const initialHash = JSON.stringify(this.initialObjects[type][object.id]); if (exportedHash !== initialHash) { splitted.updated[type].push(object); } @@ -113,24 +123,22 @@ } // Now find deleted objects - const indexes = exported.tracks.concat(exported.shapes) - .concat(exported.tags).map(object => object.id); - - for (const id of Object.keys(this.initialObjects)) { - if (!indexes.includes(+id)) { - const object = this.initialObjects[id]; - let type = null; - if ('shapes' in object) { - type = 'tracks'; - } else if ('points' in object) { - type = 'shapes'; - } else { - type = 'tags'; + const indexes = { + shapes: exported.shapes.map((object) => +object.id), + tracks: exported.tracks.map((object) => +object.id), + tags: exported.tags.map((object) => +object.id), + }; + + for (const type of Object.keys(this.initialObjects)) { + for (const id of Object.keys(this.initialObjects[type])) { + if (!indexes[type].includes(+id)) { + const object = this.initialObjects[type][id]; + splitted.deleted[type].push(object); } - splitted.deleted[type].push(object); } } + return splitted; } @@ -164,9 +172,9 @@ _receiveIndexes(exported) { // Receive client indexes before saving const indexes = { - tracks: exported.tracks.map(track => track.clientID), - shapes: exported.shapes.map(shape => shape.clientID), - tags: exported.tags.map(tag => tag.clientID), + tracks: exported.tracks.map((track) => track.clientID), + shapes: exported.shapes.map((shape) => shape.clientID), + tags: exported.tags.map((tag) => tag.clientID), }; // Remove them from the request body @@ -192,9 +200,7 @@ if (flush) { onUpdate('New objects are being saved..'); const indexes = this._receiveIndexes(exported); - const savedData = await this._put(Object.assign({}, exported, { - version: this.version, - })); + const savedData = await this._put({ ...exported, version: this.version }); this.version = savedData.version; this.collection.flush = false; @@ -202,9 +208,12 @@ this._updateCreatedObjects(savedData, indexes); onUpdate('Initial state is being updated'); - for (const object of savedData.shapes - .concat(savedData.tracks).concat(savedData.tags)) { - this.initialObjects[object.id] = object; + + this._resetState(); + for (const type of Object.keys(this.initialObjects)) { + for (const object of savedData[type]) { + this.initialObjects[type][object.id] = object; + } } } else { const { @@ -215,44 +224,41 @@ onUpdate('New objects are being saved..'); const indexes = this._receiveIndexes(created); - const createdData = await this._create(Object.assign({}, created, { - version: this.version, - })); + const createdData = await this._create({ ...created, version: this.version }); this.version = createdData.version; onUpdate('Saved objects are being updated in the client'); this._updateCreatedObjects(createdData, indexes); onUpdate('Initial state is being updated'); - for (const object of createdData.shapes - .concat(createdData.tracks).concat(createdData.tags)) { - this.initialObjects[object.id] = object; + for (const type of Object.keys(this.initialObjects)) { + for (const object of createdData[type]) { + this.initialObjects[type][object.id] = object; + } } onUpdate('Changed objects are being saved..'); this._receiveIndexes(updated); - const updatedData = await this._update(Object.assign({}, updated, { - version: this.version, - })); - this.version = createdData.version; + const updatedData = await this._update({ ...updated, version: this.version }); + this.version = updatedData.version; onUpdate('Initial state is being updated'); - for (const object of updatedData.shapes - .concat(updatedData.tracks).concat(updatedData.tags)) { - this.initialObjects[object.id] = object; + for (const type of Object.keys(this.initialObjects)) { + for (const object of updatedData[type]) { + this.initialObjects[type][object.id] = object; + } } onUpdate('Changed objects are being saved..'); this._receiveIndexes(deleted); - const deletedData = await this._delete(Object.assign({}, deleted, { - version: this.version, - })); + const deletedData = await this._delete({ ...deleted, version: this.version }); this._version = deletedData.version; onUpdate('Initial state is being updated'); - for (const object of deletedData.shapes - .concat(deletedData.tracks).concat(deletedData.tags)) { - delete this.initialObjects[object.id]; + for (const type of Object.keys(this.initialObjects)) { + for (const object of deletedData[type]) { + delete this.initialObjects[type][object.id]; + } } } diff --git a/cvat-core/tests/api/annotations.js b/cvat-core/tests/api/annotations.js index 7da68301..71a70f46 100644 --- a/cvat-core/tests/api/annotations.js +++ b/cvat-core/tests/api/annotations.js @@ -17,6 +17,7 @@ jest.mock('../../src/server-proxy', () => { // Initialize api window.cvat = require('../../src/api'); +const serverProxy = require('../../src/server-proxy'); // Test cases describe('Feature: get annotations', () => { @@ -373,6 +374,30 @@ describe('Feature: save annotations', () => { await job.annotations.save(); expect(await job.annotations.hasUnsavedChanges()).toBe(false); }); + + test('delete & save annotations for a job when there are a track and a shape with the same id', async () => { + const job = (await window.cvat.jobs.get({ jobID: 112 }))[0]; + const annotations = await job.annotations.get(0); + let okay = false; + + // Temporary override this method because we need to know what data + // have been sent to a server + const oldImplementation = serverProxy.annotations.updateAnnotations; + serverProxy.annotations.updateAnnotations = async (session, id, data, action) => { + const result = await oldImplementation + .call(serverProxy.annotations, session, id, data, action); + if (action === 'delete') { + okay = okay || (action === 'delete' && !!(data.shapes.length || data.tracks.length)); + } + return result; + }; + + await annotations[0].delete(); + await job.annotations.save(); + + serverProxy.annotations.updateAnnotations = oldImplementation; + expect(okay).toBe(true); + }); }); describe('Feature: merge annotations', () => { @@ -383,9 +408,9 @@ describe('Feature: merge annotations', () => { const states = [annotations0[0], annotations1[0]]; await task.annotations.merge(states); const merged0 = (await task.annotations.get(0)) - .filter(state => state.objectType === window.cvat.enums.ObjectType.TRACK); + .filter((state) => state.objectType === window.cvat.enums.ObjectType.TRACK); const merged1 = (await task.annotations.get(1)) - .filter(state => state.objectType === window.cvat.enums.ObjectType.TRACK); + .filter((state) => state.objectType === window.cvat.enums.ObjectType.TRACK); expect(merged0).toHaveLength(1); expect(merged1).toHaveLength(1); @@ -400,9 +425,9 @@ describe('Feature: merge annotations', () => { const states = [annotations0[0], annotations1[0]]; await job.annotations.merge(states); const merged0 = (await job.annotations.get(0)) - .filter(state => state.objectType === window.cvat.enums.ObjectType.TRACK); + .filter((state) => state.objectType === window.cvat.enums.ObjectType.TRACK); const merged1 = (await job.annotations.get(1)) - .filter(state => state.objectType === window.cvat.enums.ObjectType.TRACK); + .filter((state) => state.objectType === window.cvat.enums.ObjectType.TRACK); expect(merged0).toHaveLength(1); expect(merged1).toHaveLength(1); @@ -456,7 +481,7 @@ describe('Feature: merge annotations', () => { const task = (await window.cvat.tasks.get({ id: 100 }))[0]; const annotations0 = await task.annotations.get(0); const annotations1 = (await task.annotations.get(1)) - .filter(state => state.shapeType === window.cvat.enums.ObjectShape.POLYGON); + .filter((state) => state.shapeType === window.cvat.enums.ObjectShape.POLYGON); const states = [annotations0[0], annotations1[0]]; expect(task.annotations.merge(states)) @@ -488,7 +513,7 @@ describe('Feature: split annotations', () => { expect(annotations4[0].clientID).toBe(annotations5[0].clientID); await task.annotations.split(annotations5[0], 5); const splitted4 = await task.annotations.get(4); - const splitted5 = (await task.annotations.get(5)).filter(state => !state.outside); + const splitted5 = (await task.annotations.get(5)).filter((state) => !state.outside); expect(splitted4[0].clientID).not.toBe(splitted5[0].clientID); }); @@ -500,7 +525,7 @@ describe('Feature: split annotations', () => { expect(annotations4[0].clientID).toBe(annotations5[0].clientID); await job.annotations.split(annotations5[0], 5); const splitted4 = await job.annotations.get(4); - const splitted5 = (await job.annotations.get(5)).filter(state => !state.outside); + const splitted5 = (await job.annotations.get(5)).filter((state) => !state.outside); expect(splitted4[0].clientID).not.toBe(splitted5[0].clientID); }); diff --git a/cvat-core/tests/api/tasks.js b/cvat-core/tests/api/tasks.js index ac6560b1..232bec63 100644 --- a/cvat-core/tests/api/tasks.js +++ b/cvat-core/tests/api/tasks.js @@ -26,7 +26,7 @@ describe('Feature: get a list of tasks', () => { test('get all tasks', async () => { const result = await window.cvat.tasks.get(); expect(Array.isArray(result)).toBeTruthy(); - expect(result).toHaveLength(5); + expect(result).toHaveLength(6); for (const el of result) { expect(el).toBeInstanceOf(Task); } @@ -137,7 +137,7 @@ describe('Feature: save a task', () => { }); expect(result[0].labels).toHaveLength(labelsLength + 1); - const appendedLabel = result[0].labels.filter(el => el.name === 'My boss\'s car'); + const appendedLabel = result[0].labels.filter((el) => el.name === 'My boss\'s car'); expect(appendedLabel).toHaveLength(1); expect(appendedLabel[0].attributes).toHaveLength(1); expect(appendedLabel[0].attributes[0].name).toBe('parked'); diff --git a/cvat-core/tests/mocks/dummy-data.mock.js b/cvat-core/tests/mocks/dummy-data.mock.js index 1f215d21..e8449c06 100644 --- a/cvat-core/tests/mocks/dummy-data.mock.js +++ b/cvat-core/tests/mocks/dummy-data.mock.js @@ -185,12 +185,46 @@ const shareDummyData = [ ] const tasksDummyData = { - "count": 4, + "count": 5, "next": null, "previous": null, "results": [ { - "url": "http://localhost:7000/api/v1/tasks/1", + "url": "http://localhost:7000/api/v1/tasks/102", + "id": 102, + "name": "Test", + "size": 1, + "mode": "annotation", + "owner": 1, + "assignee": null, + "bug_tracker": "", + "created_date": "2019-09-05T11:59:22.987942Z", + "updated_date": "2019-09-05T14:04:07.569344Z", + "overlap": 0, + "segment_size": 0, + "z_order": false, + "status": "annotation", + "labels": [{ + "id": 5, + "name": "car", + "attributes": [] + }], + "segments": [{ + "start_frame": 0, + "stop_frame": 0, + "jobs": [{ + "url":"http://localhost:7000/api/v1/jobs/112", + "id": 112, + "assignee":null, + "status":"annotation" + }] + }], + "image_quality": 50, + "start_frame": 0, + "stop_frame": 0, + "frame_filter": "" + }, { + "url": "http://localhost:7000/api/v1/tasks/100", "id": 100, "name": "Image Task", "size": 9, @@ -226,7 +260,7 @@ const tasksDummyData = { "stop_frame": 8, "jobs": [ { - "url": "http://localhost:7000/api/v1/jobs/1", + "url": "http://localhost:7000/api/v1/jobs/100", "id": 100, "assignee": null, "status": "annotation" @@ -1387,6 +1421,48 @@ const tasksDummyData = { } const taskAnnotationsDummyData = { + '112': { + "version":21, + "tags": [], + "shapes": [{ + "type": "rectangle", + "occluded": false, + "z_order": 1, + "points": [ + 557.7890625, + 276.2216796875, + 907.1888732910156, + 695.5014038085938 + ], + "id": 15, + "frame": 0, + "label_id": 5, + "group": 0, + "attributes": [] + }], + "tracks": [{ + "id": 15, + "frame": 0, + "label_id": 5, + "group": 0, + "shapes": [{ + "type": "rectangle", + "occluded": false, + "z_order": 13, + "points": [ + 792.787109375, + 16.5234375, + 1171.1027526855469, + 521.3458862304688 + ], + "id": 22, + "frame": 0, + "outside": false, + "attributes": [] + }], + "attributes": [] + }] + }, '101': { "version":21, "tags":[], @@ -2514,6 +2590,10 @@ const frameMetaDummyData = { "width": 1888, "height": 1408 }], + 102: [{ + "width":1920, + "height":1080 + }], } module.exports = { diff --git a/cvat-core/tests/mocks/server-proxy.mock.js b/cvat-core/tests/mocks/server-proxy.mock.js index fd53b954..2f7ed2f5 100644 --- a/cvat-core/tests/mocks/server-proxy.mock.js +++ b/cvat-core/tests/mocks/server-proxy.mock.js @@ -287,11 +287,12 @@ class ServerProxy { }, annotations: { - value: Object.freeze({ + value: { updateAnnotations, getAnnotations, - }), - writable: false, + }, + // To implement on of important tests + writable: true, }, })); } diff --git a/cvat/apps/engine/static/engine/js/annotationSaver.js b/cvat/apps/engine/static/engine/js/annotationSaver.js index 54105191..b48f420f 100644 --- a/cvat/apps/engine/static/engine/js/annotationSaver.js +++ b/cvat/apps/engine/static/engine/js/annotationSaver.js @@ -20,22 +20,30 @@ class AnnotationSaverModel extends Listener { this._version = initialData.version; this._shapeCollection = shapeCollection; - this._initialObjects = []; + this._initialObjects = {}; + this._resetState(); this.update(); // We need use data from export instead of initialData // Otherwise we have differ keys order and JSON comparison code incorrect const data = this._shapeCollection.export()[0]; for (const shape of data.shapes) { - this._initialObjects[shape.id] = shape; + this._initialObjects.shapes[shape.id] = shape; } for (const track of data.tracks) { - this._initialObjects[track.id] = track; + this._initialObjects.tracks[track.id] = track; } } + _resetState() { + this._initialObjects = { + shapes: {}, + tracks: {}, + }; + } + update() { this._hash = this._getHash(); } @@ -122,9 +130,6 @@ class AnnotationSaverModel extends Listener { } _split(exported) { - const exportedIDs = Array.from(exported.shapes, shape => +shape.id) - .concat(Array.from(exported.tracks, track => +track.id)); - const created = { version: this._version, shapes: [], @@ -148,30 +153,36 @@ class AnnotationSaverModel extends Listener { // Compare initial state objects and export state objects // in order to get updated and created objects - for (const obj of exported.shapes.concat(exported.tracks)) { - if (obj.id in this._initialObjects) { - const exportedHash = JSON.stringify(obj); - const initialSash = JSON.stringify(this._initialObjects[obj.id]); - if (exportedHash !== initialSash) { - const target = 'shapes' in obj ? updated.tracks : updated.shapes; - target.push(obj); + for (const type of Object.keys(this._initialObjects)) { + for (const obj of exported[type]) { + if (obj.id in this._initialObjects[type]) { + const exportedHash = JSON.stringify(obj); + const initialSash = JSON.stringify(this._initialObjects[type][obj.id]); + if (exportedHash !== initialSash) { + updated[type].push(obj); + } + } else if (typeof obj.id === 'undefined') { + created[type].push(obj); + } else { + throw Error(`Bad object ID found: ${obj.id}. ` + + 'It is not contained in initial state and have server ID'); } - } else if (typeof obj.id === 'undefined') { - const target = 'shapes' in obj ? created.tracks : created.shapes; - target.push(obj); - } else { - throw Error(`Bad object ID found: ${obj.id}. ` - + 'It is not contained in initial state and have server ID'); } } + const indexes = { + shapes: exported.shapes.map((object) => +object.id), + tracks: exported.tracks.map((object) => +object.id), + }; + // Compare initial state indexes and export state indexes // in order to get removed objects - for (const shapeID in this._initialObjects) { - if (!exportedIDs.includes(+shapeID)) { - const initialShape = this._initialObjects[shapeID]; - const target = 'shapes' in initialShape ? deleted.tracks : deleted.shapes; - target.push(initialShape); + for (const type of Object.keys(this._initialObjects)) { + for (const shapeID in this._initialObjects[type]) { + if (!indexes[type].includes(+shapeID)) { + const object = this._initialObjects[type][shapeID]; + deleted[type].push(object); + } } } @@ -229,10 +240,12 @@ class AnnotationSaverModel extends Listener { this._updateCreatedObjects(exported, savedObjects, mapping); this._shapeCollection.flush = false; this._version = savedObjects.version; - for (const object of savedObjects.shapes.concat(savedObjects.tracks)) { - this._initialObjects[object.id] = object; + this._resetState(); + for (const type of Object.keys(this._initialObjects)) { + for (const object of savedObjects[type]) { + this._initialObjects[type][object.id] = object; + } } - this._version = savedObjects.version; } else { const [created, updated, deleted] = this._split(exported); @@ -240,25 +253,27 @@ class AnnotationSaverModel extends Listener { const savedCreated = await this._create(created); this._updateCreatedObjects(created, savedCreated, mapping); this._version = savedCreated.version; - for (const object of created.shapes.concat(created.tracks)) { - this._initialObjects[object.id] = object; + for (const type of Object.keys(this._initialObjects)) { + for (const object of savedCreated[type]) { + this._initialObjects[type][object.id] = object; + } } this.notify('saveUpdated'); const savedUpdated = await this._update(updated); this._version = savedUpdated.version; - for (const object of updated.shapes.concat(updated.tracks)) { - if (object.id in this._initialObjects) { - this._initialObjects[object.id] = object; + for (const type of Object.keys(this._initialObjects)) { + for (const object of savedUpdated[type]) { + this._initialObjects[type][object.id] = object; } } this.notify('saveDeleted'); const savedDeleted = await this._delete(deleted); this._version = savedDeleted.version; - for (const object of savedDeleted.shapes.concat(savedDeleted.tracks)) { - if (object.id in this._initialObjects) { - delete this._initialObjects[object.id]; + for (const type of Object.keys(this._initialObjects)) { + for (const object of savedDeleted[type]) { + delete this._initialObjects[type][object.id]; } }