Skip to content

Commit

Permalink
nodes: remove the data field from geojson collection
Browse files Browse the repository at this point in the history 
This improves the time and memory necessary to retrieve the node geojson
collection and it does not appear to have side effects.

On a large instance (all the greater Montreal area) this divides by 3
the query time to get the node geojson collection and can reduce the
memory usage in the browser by 100 MB.

Tested in the UI:

* Node edits
* Multiple node edits
* Path creation and routing
  • Loading branch information
@tahini
tahini committed Dec 9, 2024
1 parent 8dbcab7 commit d730de9
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 30 deletions.
64 changes: 35 additions & 29 deletions packages/transition-backend/src/models/db/__tests__/transitNodes.db.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,20 @@ describe(`${objectName}`, () => {

const _collection = await dbQueries.geojsonCollection();
const geojsonCollection = new GeojsonCollection(_collection.features, {}, undefined);
const _newObjectAttributes = Object.assign({}, newObjectAttributes) as any;
const _newObjectAttributes2 = Object.assign({}, newObjectAttributes2) as any;
const _newObjectAttributes = _cloneDeep(newObjectAttributes) as any;
const _newObjectAttributes2 = _cloneDeep(newObjectAttributes2) as any;
const _updatedAttributes = _cloneDeep(updatedAttributes);
const collection = geojsonCollection.features;
expect(collection.length).toBe(2);
for (const attribute in updatedAttributes) {
_newObjectAttributes[attribute] = updatedAttributes[attribute];
for (const attribute in _updatedAttributes) {
_newObjectAttributes[attribute] = _updatedAttributes[attribute];
}
delete _newObjectAttributes.data.transferableNodes;
delete _newObjectAttributes2.data.transferableNodes;
delete _newObjectAttributes.data;
delete _newObjectAttributes2.data;
delete collection[0].properties.created_at;
delete collection[0].properties.updated_at;
delete collection[0].properties.data.routingRadiusPixelsAtMaxZoom;
delete collection[1].properties.created_at;
delete collection[1].properties.updated_at;
delete collection[1].properties.data.routingRadiusPixelsAtMaxZoom;

expect(collection[0].properties.id).toBe(_newObjectAttributes.id);
expect(collection[0].properties).toMatchObject(_newObjectAttributes);
Expand All @@ -223,16 +222,16 @@ describe(`${objectName}`, () => {

const _collection = await dbQueries.geojsonCollection({ nodeIds: [newObjectAttributes.id] });
const geojsonCollection = new GeojsonCollection(_collection.features, {}, undefined);
const _newObjectAttributes = Object.assign({}, newObjectAttributes) as any;
const _newObjectAttributes = _cloneDeep(newObjectAttributes) as any;
const _updatedAttributes = _cloneDeep(updatedAttributes);
const collection = geojsonCollection.features;
expect(collection.length).toBe(1);
for (const attribute in updatedAttributes) {
_newObjectAttributes[attribute] = updatedAttributes[attribute];
for (const attribute in _updatedAttributes) {
_newObjectAttributes[attribute] = _updatedAttributes[attribute];
}
delete _newObjectAttributes.data.transferableNodes;
delete _newObjectAttributes.data;
delete collection[0].properties.created_at;
delete collection[0].properties.updated_at;
delete collection[0].properties.data.routingRadiusPixelsAtMaxZoom;

expect(collection[0].properties.id).toBe(_newObjectAttributes.id);
expect(collection[0].properties).toMatchObject(_newObjectAttributes);
Expand Down Expand Up @@ -387,45 +386,51 @@ describe(`${objectName}`, () => {

describe('Nodes, with transactions', () => {

// Copy the attributes to delete the transferableNodes for these test cases
const testObjectAttributes = _cloneDeep(newObjectAttributes);
delete (testObjectAttributes.data as any).transferableNodes;
const testObjectAttributes2 = _cloneDeep(newObjectAttributes2);
delete (testObjectAttributes2.data as any).transferableNodes;

beforeEach(async () => {
// Empty the table and add 1 object
await dbQueries.truncate();
const newObject = new ObjectClass(newObjectAttributes, true);
const newObject = new ObjectClass(testObjectAttributes, true);
await dbQueries.create(newObject.getAttributes());
});

test('Create, update with success', async() => {
const newName = 'new name';
await knex.transaction(async (trx) => {
const newObject = new ObjectClass(newObjectAttributes2, true);
const newObject = new ObjectClass(testObjectAttributes2, true);
await dbQueries.create(newObject.getAttributes(), { transaction: trx });
await dbQueries.update(newObjectAttributes.id, { name: newName }, { transaction: trx });
await dbQueries.update(testObjectAttributes.id, { name: newName }, { transaction: trx });
});

// Make sure the new object is there and the old has been updated
const collection = await dbQueries.collection();
expect(collection.length).toEqual(2);
const { name, ...currentObject } = new ObjectClass(newObjectAttributes, true).attributes;
const object1 = collection.find((obj) => obj.id === newObjectAttributes.id);
const { name, ...currentObject } = new ObjectClass(testObjectAttributes, true).attributes;
const object1 = collection.find((obj) => obj.id === testObjectAttributes.id);
expect(object1).toBeDefined();
expect(object1).toEqual(expect.objectContaining({
name: newName,
...currentObject
}));

const object2 = collection.find((obj) => obj.id === newObjectAttributes2.id);
const object2 = collection.find((obj) => obj.id === testObjectAttributes2.id);
expect(object2).toBeDefined();
expect(object2).toEqual(expect.objectContaining(new ObjectClass(newObjectAttributes2, true).attributes));
expect(object2).toEqual(expect.objectContaining(new ObjectClass(testObjectAttributes2, true).attributes));
});

test('Create, update with error', async() => {
let error: any = undefined;
try {
await knex.transaction(async (trx) => {
const newObject = new ObjectClass(newObjectAttributes2, true);
const newObject = new ObjectClass(testObjectAttributes2, true);
await dbQueries.create(newObject.getAttributes(), { transaction: trx });
// Update with a bad field
await dbQueries.update(newObjectAttributes.id, { simulation_id: uuidV4() } as any, { transaction: trx });
await dbQueries.update(testObjectAttributes.id, { simulation_id: uuidV4() } as any, { transaction: trx });
});
} catch(err) {
error = err;
Expand All @@ -435,20 +440,20 @@ describe('Nodes, with transactions', () => {
// The new object should not have been added and the one in DB should not have been updated
const collection = await dbQueries.collection();
expect(collection.length).toEqual(1);
const object1 = collection.find((obj) => obj.id === newObjectAttributes.id);
const object1 = collection.find((obj) => obj.id === testObjectAttributes.id);
expect(object1).toBeDefined();
expect(object1).toEqual(expect.objectContaining(new ObjectClass(newObjectAttributes, true).attributes));
expect(object1).toEqual(expect.objectContaining(new ObjectClass(testObjectAttributes, true).attributes));
});

test('Create, update, delete with error', async() => {
const currentNodeNewName = 'new node name';
let error: any = undefined;
try {
await knex.transaction(async (trx) => {
const newObject = new ObjectClass(newObjectAttributes2, true);
const newObject = new ObjectClass(testObjectAttributes2, true);
await dbQueries.create(newObject.getAttributes(), { transaction: trx });
await dbQueries.update(newObjectAttributes.id, { name: currentNodeNewName }, { transaction: trx });
await dbQueries.delete(newObjectAttributes.id, { transaction: trx });
await dbQueries.update(testObjectAttributes.id, { name: currentNodeNewName }, { transaction: trx });
await dbQueries.delete(testObjectAttributes.id, { transaction: trx });
throw 'error';
});
} catch(err) {
Expand All @@ -459,9 +464,10 @@ describe('Nodes, with transactions', () => {
// Make sure the existing object is still there and no new one has been added
const collection = await dbQueries.collection();
expect(collection.length).toEqual(1);
const object1 = collection.find((obj) => obj.id === newObjectAttributes.id);
const object1 = collection.find((obj) => obj.id === testObjectAttributes.id);
expect(object1).toBeDefined();
expect(object1).toEqual(expect.objectContaining(new ObjectClass(newObjectAttributes, true).attributes));
expect(object1).toEqual(expect.objectContaining(new ObjectClass(testObjectAttributes, true).attributes));
});

});
newObjectAttributes
1 change: 0 additions & 1 deletion packages/transition-backend/src/models/db/transitNodes.db.queries.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ const geojsonCollection = async (
'name', name,
'description', description,
'geography', ST_AsGeoJSON(geography)::jsonb,
'data', data,
'is_enabled', is_enabled,
'integer_id', integer_id,
'routing_radius_meters', routing_radius_meters,
Expand Down

0 comments on commit d730de9

Please sign in to comment.