From 8ce1d5ea89264e5e008824770ec709fd401baeb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Bastien?= Date: Fri, 18 Oct 2024 14:38:35 -0400 Subject: [PATCH 1/4] db: Add a `transaction` option to `exists` and `read` functions If a read/exists call is done within a transaction on an element that was added/modified during the transaction, if the transaction parameter is not set, the object will not be correctly read. This lets elements calls inside transaction to read the data as it is in the transaction. --- .../src/models/db/default.db.queries.ts | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/packages/chaire-lib-backend/src/models/db/default.db.queries.ts b/packages/chaire-lib-backend/src/models/db/default.db.queries.ts index dd85a1b69..aa11e39fa 100644 --- a/packages/chaire-lib-backend/src/models/db/default.db.queries.ts +++ b/packages/chaire-lib-backend/src/models/db/default.db.queries.ts @@ -40,9 +40,19 @@ export const stringifyDataSourceIds = function (dataSourceIds: string[]): string * @param knex The database configuration object * @param tableName The name of the table on which to execute the operation * @param id The ID of the object + * @param {Object} options Additional options parameter. + * @param {Knex.Transaction} [options.transaction] - transaction that this query + * is part of * @returns Whether an object with the given ID exists in the table */ -export const exists = async (knex: Knex, tableName: string, id: string): Promise => { +export const exists = async ( + knex: Knex, + tableName: string, + id: string, + options: { + transaction?: Knex.Transaction; + } = {} +): Promise => { if (!uuidValidate(id)) { throw new TrError( `Cannot verify if the object exists in ${tableName} because the required parameter id is missing, blank or not a valid uuid`, @@ -51,7 +61,11 @@ export const exists = async (knex: Knex, tableName: string, id: string): Promise ); } try { - const rows = await knex(tableName).count('*').where('id', id); + const query = knex(tableName).count('*').where('id', id); + if (options.transaction) { + query.transacting(options.transaction); + } + const rows = await query; const count = rows.length > 0 ? rows[0].count : 0; if (count) { @@ -177,9 +191,12 @@ export const createMultiple = async ( * @param tableName The name of the table on which to execute the operation * @param parser A parser function which converts the database fields to an * object's attributes - * @param query The raw select fields to query. Defaults to `*` to read all + * @param select The raw select fields to query. Defaults to `*` to read all * fields in the table * @param id The ID of the object to read + * @param {Object} options Additional options parameter. + * @param {Knex.Transaction} [options.transaction] - transaction that this query + * is part of * @returns The object attributes obtained after the `parser` function was run * on the read record. */ @@ -187,8 +204,11 @@ export const read = async ( knex: Knex, tableName: string, parser: ((arg: U) => Partial) | undefined, - query = '*', - id: string + select = '*', + id: string, + options: { + transaction?: Knex.Transaction; + } = {} ): Promise> => { try { if (!uuidValidate(id)) { @@ -198,7 +218,11 @@ export const read = async ( 'DatabaseCannotReadBecauseIdIsMissingOrInvalid' ); } - const rows = await knex(tableName).select(knex.raw(query)).where('id', id); + const query = knex(tableName).select(knex.raw(select)).where('id', id); + if (options.transaction) { + query.transacting(options.transaction); + } + const rows = await query; if (rows.length !== 1) { throw new TrError( From 0168a739687e60fa562aa61bf45b093b34e32a4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Bastien?= Date: Fri, 18 Oct 2024 14:42:12 -0400 Subject: [PATCH 2/4] schedules in db: Deprecate the `create` and `update` db functions The `save` function already covers both functionality more transparently than the caller having to select between `create` and `update`, so this should be called instead. But we can't remove it since currently the TransitObjectHandler use the create and update functions. But all other calls have been changed. --- .../db/__tests__/TransitPath.db.test.ts | 2 +- .../db/__tests__/TransitSchedule.db.test.ts | 41 ++++--------------- .../db/__tests__/transitLines.db.test.ts | 4 +- .../db/__tests__/transitServices.db.test.ts | 2 +- .../models/db/transitSchedules.db.queries.ts | 17 ++++++-- .../services/gtfsImport/ScheduleImporter.ts | 2 +- .../__tests__/ScheduleImporter.test.ts | 41 ++++++++----------- 7 files changed, 43 insertions(+), 66 deletions(-) diff --git a/packages/transition-backend/src/models/db/__tests__/TransitPath.db.test.ts b/packages/transition-backend/src/models/db/__tests__/TransitPath.db.test.ts index 4c89bfeb7..021faf3c4 100644 --- a/packages/transition-backend/src/models/db/__tests__/TransitPath.db.test.ts +++ b/packages/transition-backend/src/models/db/__tests__/TransitPath.db.test.ts @@ -264,7 +264,7 @@ describe(`${objectName}`, function() { }], "periods_group_shortname": "all_day", } as any; - await schedulesDbQueries.create(scheduleForServiceId); + await schedulesDbQueries.save(scheduleForServiceId); const geojsonCollection = await dbQueries.geojsonCollection({ scenarioId }) expect(geojsonCollection.features.length).toBe(1); diff --git a/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts b/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts index 31168713d..85f1c7293 100644 --- a/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts +++ b/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts @@ -219,7 +219,7 @@ describe(`schedules`, function () { expect(scheduleDataValidation.isValid).toBe(true); expect(ScheduleDataValidator.validate(scheduleForServiceId, pathStub1).isValid).toBe(true); expect(ScheduleDataValidator.validate(scheduleForServiceId, pathStub2).isValid).toBe(false); - const newId = await dbQueries.create(scheduleForServiceId as any); + const newId = await dbQueries.save(scheduleForServiceId as any); expect(newId).toBe(scheduleForServiceId.id); }); @@ -231,7 +231,7 @@ describe(`schedules`, function () { existingServiceLineSchedule.periods = []; let exception: any = undefined; try { - await dbQueries.create(existingServiceLineSchedule as any); + await dbQueries.save(existingServiceLineSchedule as any); } catch(error) { exception = error; } @@ -274,7 +274,7 @@ describe(`schedules`, function () { updatedSchedule.periods[1].trips[0].seated_capacity = 30; // Update the object - const newId = await dbQueries.update(updatedSchedule.id, updatedSchedule); + const newId = await dbQueries.save(updatedSchedule); expect(newId).toBe(scheduleForServiceId.id); // Delete the updated_at fields @@ -303,7 +303,7 @@ describe(`schedules`, function () { updatedSchedule.periods[0].trips.splice(2, 1); // Update the object - const newId = await dbQueries.update(updatedSchedule.id, updatedSchedule); + const newId = await dbQueries.save(updatedSchedule); expect(newId).toBe(scheduleForServiceId.id); // Expect anything for the updated_at fields @@ -336,7 +336,7 @@ describe(`schedules`, function () { updatedSchedule.periods[0].trips.push(newTrip as any); // Update the object - const newId = await dbQueries.update(updatedSchedule.id, updatedSchedule); + const newId = await dbQueries.save(updatedSchedule); expect(newId).toBe(scheduleForServiceId.id); // Expect anything for the updated_at fields @@ -363,29 +363,6 @@ describe(`schedules`, function () { }); - test('Test save method it should create or update the object', async () => { - - // Clone the object, then save, it should be created - const objectCopy = _cloneDeep(scheduleForServiceId) as any; - - let newId = await dbQueries.save(objectCopy); - expect(newId).toBe(scheduleForServiceId.id); - - // Read the object from DB - let scheduleDataRead = await dbQueries.read(scheduleForServiceId.id); - expect(scheduleDataRead).toMatchObject(objectCopy); - - // Update the object, then save and read again - objectCopy.allow_seconds_based_schedules = true; - newId = await dbQueries.save(objectCopy); - expect(newId).toBe(scheduleForServiceId.id); - - // Read the object from DB - scheduleDataRead = await dbQueries.read(scheduleForServiceId.id); - expect(scheduleDataRead).toMatchObject(objectCopy); - - }); - }); describe('Schedules, single queries with transaction errors', () => { @@ -411,7 +388,7 @@ describe('Schedules, single queries with transaction errors', () => { test('update with periods and trips, with error', async() => { // Insert the schedule - await dbQueries.create(scheduleForServiceId as any); + await dbQueries.save(scheduleForServiceId as any); // Read the object from DB and make sure it has not changed const originalData = await dbQueries.read(scheduleForServiceId.id); @@ -450,7 +427,7 @@ describe('Schedules, with transactions', () => { await dbQueries.save(originalSchedule, { transaction: trx }); // Save the updated schedule with one less period and trip - await dbQueries.update(updatedSchedule.id, updatedSchedule, { transaction: trx }); + await dbQueries.save(updatedSchedule, { transaction: trx }); }); // Make sure the object is there and updated @@ -474,7 +451,7 @@ describe('Schedules, with transactions', () => { await dbQueries.save(originalSchedule, { transaction: trx }); // Save the updated schedule with one less period and trip - await dbQueries.update(updatedSchedule.id, updatedSchedule, { transaction: trx }); + await dbQueries.save(updatedSchedule, { transaction: trx }); }); } catch(err) { error = err; @@ -500,7 +477,7 @@ describe('Schedules, with transactions', () => { try { await knex.transaction(async (trx) => { // Update, then delete the schedule, then throw an error - await dbQueries.update(updatedSchedule.id, updatedSchedule, { transaction: trx }); + await dbQueries.save(updatedSchedule, { transaction: trx }); await dbQueries.delete(scheduleForServiceId.id, { transaction: trx }); throw 'error'; }); diff --git a/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts b/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts index 080195279..c0d162a87 100644 --- a/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts +++ b/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts @@ -221,8 +221,8 @@ describe(`${objectName}`, () => { const _updatedAttributes = Object.assign({}, newObjectAttributesWithSchedule); const updatedObject = new ObjectClass(_updatedAttributes, false); const id = await dbQueries.update(newObjectAttributesWithSchedule.id, updatedObject.getAttributes()); - await schedulesDbQueries.create(scheduleForServiceId); - await schedulesDbQueries.create(scheduleForServiceId2); + await schedulesDbQueries.save(scheduleForServiceId); + await schedulesDbQueries.save(scheduleForServiceId2); await pathsDbQueries.create(pathAttributes); expect(id).toBe(newObjectAttributesWithSchedule.id); diff --git a/packages/transition-backend/src/models/db/__tests__/transitServices.db.test.ts b/packages/transition-backend/src/models/db/__tests__/transitServices.db.test.ts index 0b4331959..1190f6cfd 100644 --- a/packages/transition-backend/src/models/db/__tests__/transitServices.db.test.ts +++ b/packages/transition-backend/src/models/db/__tests__/transitServices.db.test.ts @@ -163,7 +163,7 @@ describe(`${objectName}`, function() { expect(id).toBe(newObjectAttributes2.id); // Also create a schedule - await schedulesDbQueries.create({ + await schedulesDbQueries.save({ id: uuidV4(), line_id: lineId, service_id: newObjectAttributes2.id, diff --git a/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts b/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts index a510a562e..f91464681 100644 --- a/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts +++ b/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts @@ -356,7 +356,7 @@ const updateFromScheduleData = async ( }; const save = async function (scheduleData: ScheduleAttributes, options: { transaction?: Knex.Transaction } = {}) { - const scheduleExists = await exists(knex, scheduleTable, scheduleData.id); + const scheduleExists = await exists(knex, scheduleTable, scheduleData.id, { transaction: options.transaction }); return scheduleExists ? await updateFromScheduleData(scheduleData.id, scheduleData, options) : await createFromScheduleData(scheduleData, options); @@ -515,9 +515,18 @@ export default { exists: exists.bind(null, knex, scheduleTable), read: readScheduleData, readForLine, - create: createFromScheduleData, - /** Use the 'save' method instead as it will either create or update if necessary */ - update: updateFromScheduleData, + /** + * TODO This function needs to remain for now as TransitObjectHandler makes use of it + * @deprecated Use the `save` function instead */ + create: save, + /** + * TODO This function needs to remain for now as TransitObjectHandler makes use of it + * @deprecated Use the `save` function instead */ + update: ( + scheduleId: string, + scheduleData: ScheduleAttributes, + { transaction }: { transaction?: Knex.Transaction } = {} + ) => save({ ...scheduleData, id: scheduleId }, { transaction }), save, delete: deleteScheduleData, getScheduleIdsForLine, diff --git a/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts b/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts index 6e9331a62..63e78d197 100644 --- a/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts +++ b/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts @@ -135,7 +135,7 @@ const generateAndImportSchedules = async ( await linesDbQueries.update(line.getId(), line.attributes, { returning: 'id' }); // Save schedules for line const saveSchedPromises = newSchedules.map((schedule) => { - scheduleDbQueries.create(schedule.attributes); + scheduleDbQueries.save(schedule.attributes); }); await Promise.all(saveSchedPromises); await lineObjectToCache(line); diff --git a/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts b/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts index a742c677f..c2177773c 100644 --- a/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts +++ b/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts @@ -298,15 +298,11 @@ describe('Generate schedules for lines', () => { importData.periodsGroup = testPeriod; beforeEach(() => { - (linesDbQueries.update as any).mockClear(); - (linesDbQueries.updateMultiple as any).mockClear(); - (schedulesDbQueries.create as any).mockClear(); - (schedulesDbQueries.delete as any).mockClear(); + jest.clearAllMocks(); // Reset line to its original state line = new Line({id: importData.lineIdsByRouteGtfsId[routeId], mode: 'metro', category: 'A', agency_id: uuidV4() }, false); lineCollection = new LineCollection([line], {}); collectionManager.set('lines', lineCollection); - objectToCacheMock.mockClear(); }); test('One line, simple trips, second with various pick_up/drop', async () => { @@ -341,7 +337,7 @@ describe('Generate schedules for lines', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(1); const scheduleAttributes = modifiedLine.getAttributes().scheduleByServiceId[importData.serviceIdsByGtfsId[gtfsServiceId]]; @@ -422,7 +418,7 @@ describe('Generate schedules for lines', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(1); const scheduleAttributes = modifiedLine.getAttributes().scheduleByServiceId[importData.serviceIdsByGtfsId[gtfsServiceId]]; @@ -516,7 +512,7 @@ describe('Generate schedules for lines', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(2); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(2); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(2); const scheduleAttributes = modifiedLine.getAttributes().scheduleByServiceId[importData.serviceIdsByGtfsId[gtfsServiceId]]; @@ -620,7 +616,7 @@ describe('Generate schedules for lines', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(1); @@ -640,7 +636,7 @@ describe('Generate schedules for lines', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(2); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(2); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(2); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(2); @@ -769,7 +765,7 @@ describe('Generate schedules for lines', () => { expect(linesDbQueries.update).toHaveBeenCalledTimes(2); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[expressRouteId], modifiedLine2.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(2); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(2); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(1); const scheduleAttributes = modifiedLine.getAttributes().scheduleByServiceId[importData.serviceIdsByGtfsId[gtfsServiceId]]; @@ -888,12 +884,12 @@ describe('Generate schedules for lines', () => { if (shouldUpdateSchedule) { expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(localImportData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(schedulesDbQueries.delete).toHaveBeenCalledTimes(1); expect(modifiedLine.getAttributes().scheduleByServiceId[serviceId].id).not.toEqual(previousScheduleId); } else { expect(linesDbQueries.update).toHaveBeenCalledTimes(1); - expect(schedulesDbQueries.create).not.toHaveBeenCalled(); + expect(schedulesDbQueries.save).not.toHaveBeenCalled(); expect(schedulesDbQueries.delete).not.toHaveBeenCalled(); expect(modifiedLine.getAttributes().scheduleByServiceId[serviceId].id).toEqual(previousScheduleId); } @@ -924,7 +920,7 @@ describe('Generate schedules for lines', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).not.toHaveBeenCalled(); + expect(schedulesDbQueries.save).not.toHaveBeenCalled(); expect(Object.keys(modifiedLine.getAttributes().scheduleByServiceId).length).toEqual(0); }); @@ -948,16 +944,11 @@ describe('Generate frequency based schedules for line', () => { const generateForPeriodMock = Schedule.prototype.generateForPeriod as jest.MockedFunction; beforeEach(() => { - (linesDbQueries.update as any).mockClear(); - (linesDbQueries.updateMultiple as any).mockClear(); - (schedulesDbQueries.create as any).mockClear(); - (schedulesDbQueries.delete as any).mockClear(); + jest.clearAllMocks(); // Reset line to its original state line = new Line({id: importData.lineIdsByRouteGtfsId[routeId], mode: 'metro', category: 'A', agency_id: uuidV4() }, false); lineCollection = new LineCollection([line], {}); collectionManager.set('lines', lineCollection); - objectToCacheMock.mockClear(); - generateForPeriodMock.mockClear(); }); test('Single trip for one period', async () => { @@ -984,7 +975,7 @@ describe('Generate frequency based schedules for line', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); // One period should have been generated, no trip in the other expect(generateForPeriodMock).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[0].shortname); @@ -1054,7 +1045,7 @@ describe('Generate frequency based schedules for line', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledTimes(2); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[0].shortname); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[1].shortname); @@ -1141,7 +1132,7 @@ describe('Generate frequency based schedules for line', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledTimes(2); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[0].shortname); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[1].shortname); @@ -1224,7 +1215,7 @@ describe('Generate frequency based schedules for line', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[0].shortname); @@ -1315,7 +1306,7 @@ describe('Generate frequency based schedules for line', () => { expect(modifiedLine).toBeDefined(); expect(linesDbQueries.update).toHaveBeenCalledTimes(1); expect(linesDbQueries.update).toHaveBeenCalledWith(importData.lineIdsByRouteGtfsId[routeId], modifiedLine.getAttributes(), expect.anything()); - expect(schedulesDbQueries.create).toHaveBeenCalledTimes(1); + expect(schedulesDbQueries.save).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledTimes(1); expect(generateForPeriodMock).toHaveBeenCalledWith(testPeriod.periods[0].shortname); From 38e352a925800b13dbea82c9d357a31b4092b288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Bastien?= Date: Fri, 18 Oct 2024 14:47:14 -0400 Subject: [PATCH 3/4] default db: Support ID parameter either string or number As some tables will migrate to numeric IDs and given that the knex queries automatically deal with parameter types, we can safely support either string or number IDs in the default DB functions. If it is a string, it is expected to be a uuid and we still validate its type. --- .../src/models/db/default.db.queries.ts | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/packages/chaire-lib-backend/src/models/db/default.db.queries.ts b/packages/chaire-lib-backend/src/models/db/default.db.queries.ts index aa11e39fa..3e546bc68 100644 --- a/packages/chaire-lib-backend/src/models/db/default.db.queries.ts +++ b/packages/chaire-lib-backend/src/models/db/default.db.queries.ts @@ -39,7 +39,8 @@ export const stringifyDataSourceIds = function (dataSourceIds: string[]): string * * @param knex The database configuration object * @param tableName The name of the table on which to execute the operation - * @param id The ID of the object + * @param {string|number} id The ID of the object, numeric for tables that have + * numeric primary keys, or uuid strings for tables that have uuid primary keys * @param {Object} options Additional options parameter. * @param {Knex.Transaction} [options.transaction] - transaction that this query * is part of @@ -48,12 +49,12 @@ export const stringifyDataSourceIds = function (dataSourceIds: string[]): string export const exists = async ( knex: Knex, tableName: string, - id: string, + id: string | number, options: { transaction?: Knex.Transaction; } = {} ): Promise => { - if (!uuidValidate(id)) { + if (typeof id === 'string' && !uuidValidate(id)) { throw new TrError( `Cannot verify if the object exists in ${tableName} because the required parameter id is missing, blank or not a valid uuid`, 'DBQEX0001', @@ -193,7 +194,9 @@ export const createMultiple = async ( * object's attributes * @param select The raw select fields to query. Defaults to `*` to read all * fields in the table - * @param id The ID of the object to read + * @param {string|number} id The ID of the object to fetch, numeric for tables + * that have numeric primary keys, or uuid strings for tables that have uuid + * primary keys * @param {Object} options Additional options parameter. * @param {Knex.Transaction} [options.transaction] - transaction that this query * is part of @@ -205,13 +208,13 @@ export const read = async ( tableName: string, parser: ((arg: U) => Partial) | undefined, select = '*', - id: string, + id: string | number, options: { transaction?: Knex.Transaction; } = {} ): Promise> => { try { - if (!uuidValidate(id)) { + if (typeof id === 'string' && !uuidValidate(id)) { throw new TrError( `Cannot read object from table ${tableName} because the required parameter id is missing, blank or not a valid uuid`, 'DBQRD0001', @@ -250,7 +253,9 @@ export const read = async ( * @param tableName The name of the table on which to execute the operation * @param parser A parser function which converts an object's attributes to db * fields - * @param id The ID of the record to update + * @param {string|number} id The ID of the object to update, numeric for tables + * that have numeric primary keys, or uuid strings for tables that have uuid + * primary keys * @param attributes A subset of the object's attributes to update * @param options Additional options parameter. `returning` specifies which * field's or fields' values to return after update. `transaction` is an @@ -261,15 +266,15 @@ export const update = async ( knex: Knex, tableName: string, parser: ((arg: Partial) => U) | undefined, - id: string, + id: string | number, attributes: Partial, options: { returning?: string; transaction?: Knex.Transaction; } = {} -): Promise => { +): Promise => { try { - if (!uuidValidate(id)) { + if (typeof id === 'string' && !uuidValidate(id)) { throw new TrError( `Cannot update object with id ${id} from table ${tableName} because the required parameter id is missing, blank or not a valid uuid`, 'DBQUP0001', @@ -361,7 +366,9 @@ export const updateMultiple = async ( * * @param knex The database configuration object * @param tableName The name of the table on which to execute the operation - * @param id The ID of the record to delete + * @param {string|number} id The ID of the record to delete, numeric for tables + * that have numeric primary keys, or uuid strings for tables that have uuid + * primary keys * @param options Additional options parameter. `transaction` is an optional * transaction of which this delete is part of. * @returns The ID of the deleted object @@ -369,13 +376,13 @@ export const updateMultiple = async ( export const deleteRecord = async ( knex: Knex, tableName: string, - id: string, + id: string | number, options: { transaction?: Knex.Transaction; } = {} ) => { try { - if (!uuidValidate(id)) { + if (typeof id === 'string' && !uuidValidate(id)) { throw new TrError( `Cannot verify if object exists in table ${tableName} because the required parameter id is missing, blank or not a valid uuid`, 'DBQDL0001', @@ -402,7 +409,9 @@ export const deleteRecord = async ( * * @param knex The database configuration object * @param tableName The name of the table on which to execute the operation - * @param ids An array of record IDs to delete + * @param {string[]|number[]} ids An array of record IDs to delete, numeric for + * tables that have numeric primary keys, or uuid strings for tables that have + * uuid primary keys * @param options Additional options parameter. `transaction` is an optional * transaction of which this delete is part of. * @returns The array of deleted IDs @@ -410,11 +419,11 @@ export const deleteRecord = async ( export const deleteMultiple = async ( knex: Knex, tableName: string, - ids: string[], + ids: string[] | number[], options: { transaction?: Knex.Transaction; } = {} -): Promise => { +): Promise => { try { const query = knex(tableName).whereIn('id', ids).del(); if (options.transaction) { From e622a2bd5e31d76a1f5fb144079670b3c2e49d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Bastien?= Date: Mon, 21 Oct 2024 12:57:54 -0400 Subject: [PATCH 4/4] schedules: Use numeric ids for schedules tables primary keys Change the schedule, schedule_periods and schedule_period_trips's primary id columns from uuid to auto-increment numeric IDs. The old uuid is kept is a column called `uuid`. The foreign keys are modified to use the numeric key instead. The `schedule_id` column of the trips' table is removed, as there is already a schedule_period_id foreign which has a foreign key to the schedule_id. Having both in trips open the door to data integrity issues where the period could not belong to the same schedule as the trip. auto-increment numeric IDs are more performant than uuids as keys. It will also allow to more easily duplicate multiple elements in a table with predictable sort order, that uuids do not allow. This commit only changes the database table queries. The Schedule class still use the uuid as `id` as this would require more refactoring of the base classes, history tracker, forms and client/server messaging, that will be done in later commits. The numeric IDs are kept in the `integer_id` fields of the Schedule. The foreign key are changed to map to the numeric ID. The impact is "minimal" as the schedules are used mostly in calculations by trRouting and, in the frontend, are [re-]loaded on demand when changes occur on a specific line. There is no collection of this object type where the change of ID could cause issues. --- .../capnpCache/transitLines.cache.queries.ts | 2 +- .../db/__tests__/TransitSchedule.db.test.ts | 197 ++++++++++++------ .../db/__tests__/transitLines.db.test.ts | 6 +- ...03145700_useNumericIdsInSchedulesTables.ts | 158 ++++++++++++++ .../src/models/db/transitPaths.db.queries.ts | 7 +- .../models/db/transitSchedules.db.queries.ts | 172 ++++++++++----- .../preparation/ServicePreparation.ts | 2 +- .../__tests__/ScheduleExporter.test.ts | 33 +-- .../services/gtfsImport/ScheduleImporter.ts | 14 +- .../__tests__/ScheduleImporter.test.ts | 85 ++++---- .../src/services/schedules/Schedule.ts | 9 +- .../services/schedules/ScheduleDuplicator.ts | 8 +- .../schedules/__tests__/Schedule.test.ts | 4 +- .../schedules/__tests__/ScheduleData.test.ts | 27 ++- .../__tests__/ScheduleDuplicator.test.ts | 17 +- 15 files changed, 512 insertions(+), 229 deletions(-) create mode 100644 packages/transition-backend/src/models/db/migrations/20241003145700_useNumericIdsInSchedulesTables.ts diff --git a/packages/transition-backend/src/models/capnpCache/transitLines.cache.queries.ts b/packages/transition-backend/src/models/capnpCache/transitLines.cache.queries.ts index 181183d3a..a6903ee61 100644 --- a/packages/transition-backend/src/models/capnpCache/transitLines.cache.queries.ts +++ b/packages/transition-backend/src/models/capnpCache/transitLines.cache.queries.ts @@ -76,7 +76,7 @@ const importParser = function (cacheObject: CacheObjectClass) { const periodShortname = periodCache.getPeriodShortname(); const period: Partial = { period_shortname: periodShortname, - schedule_id: schedule.id, + schedule_id: schedule.integer_id, outbound_path_id: periodCache.getOutboundPathUuid(), inbound_path_id: _emptyStringToNull(periodCache.getInboundPathUuid()), custom_start_at_str: diff --git a/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts b/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts index 85f1c7293..d075a6dcc 100644 --- a/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts +++ b/packages/transition-backend/src/models/db/__tests__/TransitSchedule.db.test.ts @@ -81,53 +81,55 @@ const pathStub2 = { } }; +let scheduleIntegerId: number | undefined = undefined; const scheduleForServiceId = { "allow_seconds_based_schedules": false, "id": "cab32276-3181-400e-a07c-719326be1f02", + integer_id: undefined, "line_id": lineId, "service_id": serviceId, "is_frozen": false, "periods": [{ // Period with start and end hours and multiple trips - "custom_start_at_str": null, + integer_id: undefined, + id: uuidV4(), "end_at_hour": 12, - "inbound_path_id": null, "interval_seconds": 1800, - "number_of_units": null, "outbound_path_id": pathId, "period_shortname": "all_day_period_shortname", "start_at_hour": 7, "trips": [{ + integer_id: undefined, "arrival_time_seconds": 27015, "block_id": "a2cadcb8-ee17-4bd7-9e77-bd400ad73064", "departure_time_seconds": 25200, "id": "42cadcb8-ee17-4bd7-9e77-bd400ad73064", - "node_arrival_times_seconds": [null, 25251, 26250, 27015], - "node_departure_times_seconds": [25200, 25261, 26260, null], + "node_arrival_times_seconds": [null, 25251, 26250, 27015] as any, + "node_departure_times_seconds": [25200, 25261, 26260, null] as any, "nodes_can_board": [true, true, true, false], "nodes_can_unboard": [false, true, true, true], "path_id": pathId, "seated_capacity": 20, "total_capacity": 50 }, { + integer_id: undefined, "arrival_time_seconds": 32416, - "block_id": null, "departure_time_seconds": 30601, "id": "5389b983-511e-4184-8776-ebc108cebaa2", - "node_arrival_times_seconds": [null, 30652, 31650, 32416], - "node_departure_times_seconds": [30601, 30662, 31660, null], + "node_arrival_times_seconds": [null, 30652, 31650, 32416] as any, + "node_departure_times_seconds": [30601, 30662, 31660, null] as any, "nodes_can_board": [true, true, true, false], "nodes_can_unboard": [false, true, true, true], "path_id": pathId, "seated_capacity": 20, "total_capacity": 50 }, { + integer_id: undefined, "arrival_time_seconds": 34216, - "block_id": null, "departure_time_seconds": 32401, "id": "448544ae-60d1-4d5b-8734-d031332cb6bc", - "node_arrival_times_seconds": [null, 32452, 33450, 34216], - "node_departure_times_seconds": [32401, 32462, 33460, null], + "node_arrival_times_seconds": [null, 32452, 33450, 34216] as any, + "node_departure_times_seconds": [32401, 32462, 33460, null] as any, "nodes_can_board": [true, true, true, false], "nodes_can_unboard": [false, true, true, true], "path_id": pathId, @@ -136,22 +138,22 @@ const scheduleForServiceId = { }] }, { // Period with custom start and end, with a single trip + integer_id: undefined, + id: uuidV4(), "custom_start_at_str": "13:15", "custom_end_at_str": "17:24", "end_at_hour": 18, - "inbound_path_id": null, "interval_seconds": 1800, - "number_of_units": null, "outbound_path_id": pathId, "period_shortname": "all_day_custom_period", "start_at_hour": 13, "trips": [{ + integer_id: undefined, "arrival_time_seconds": 50000, - "block_id": null, "departure_time_seconds": 48000, "id": "448544ae-cafe-4d5b-8734-d031332cb6bc", - "node_arrival_times_seconds": [null, 48050, 49450, 50000], - "node_departure_times_seconds": [48000, 48060, 49460, null], + "node_arrival_times_seconds": [null, 48050, 49450, 50000] as any, + "node_departure_times_seconds": [48000, 48060, 49460, null] as any, "nodes_can_board": [true, true, true, false], "nodes_can_unboard": [false, true, true, true], "path_id": pathId, @@ -160,12 +162,12 @@ const scheduleForServiceId = { }] }, { // Period with custom start and end, without trips + integer_id: undefined, + id: uuidV4(), "custom_start_at_str": "18:00", "custom_end_at_str": "23:00", "end_at_hour": 23, - "inbound_path_id": null, "interval_seconds": 1800, - "number_of_units": null, "outbound_path_id": pathId, "period_shortname": "all_day_custom_period", "start_at_hour": 18 @@ -177,12 +179,15 @@ const scheduleForServiceId = { * to debug failed test than a matchObject on the scheduleAttributes */ const expectSchedulesSame = (actual: ScheduleAttributes, expected: ScheduleAttributes) => { const { periods, ...scheduleAttributes } = actual; - const { periods: expectedPeriods, ...expectedScheduleAttributes } = expected; + const { periods: expectedPeriods, integer_id, ...expectedScheduleAttributes } = expected; expect(scheduleAttributes).toEqual(expect.objectContaining(expectedScheduleAttributes)); + if (integer_id !== undefined) { + expect(scheduleAttributes.integer_id).toEqual(integer_id); + } // Make sure all expected periods are there for (let periodIdx = 0; periodIdx < expectedPeriods.length; periodIdx++) { // Find the matching period - const { trips: expectedTrips, ...expectedPeriodAttributes } = expectedPeriods[periodIdx]; + const { trips: expectedTrips, integer_id: periodIntId, ...expectedPeriodAttributes } = expectedPeriods[periodIdx]; const matchingPeriod = periods.find(period => period.id === expectedPeriodAttributes.id); expect(matchingPeriod).toBeDefined(); // Validate period attributes @@ -192,11 +197,18 @@ const expectSchedulesSame = (actual: ScheduleAttributes, expected: ScheduleAttri continue; } expect(periodAttributes).toEqual(expect.objectContaining(expectedPeriodAttributes)); + if (periodIntId !== undefined) { + expect(periodAttributes.integer_id).toEqual(periodIntId); + } // Make sure all expected trips are there for (let tripIdx = 0; tripIdx < expectedTrips.length; tripIdx++) { const matchingTrip = trips.find(trip => trip.id === expectedTrips[tripIdx].id); expect(matchingTrip).toBeDefined(); - expect(matchingTrip).toEqual(expect.objectContaining(expectedTrips[tripIdx])); + const { integer_id: tripIntId, ...expectedTripAttributes } = expectedTrips[tripIdx]; + expect(matchingTrip).toEqual(expect.objectContaining(expectedTripAttributes)); + if (tripIntId !== undefined) { + expect(matchingTrip!.integer_id).toEqual(tripIntId); + } } expect(trips.length).toEqual(expectedTrips.length); } @@ -208,7 +220,7 @@ describe(`schedules`, function () { test('schedule exists should return false if object is not in database', async function () { // Check unexisting schedule - const exists = await dbQueries.exists(uuidV4()); + const exists = await dbQueries.exists(1); expect(exists).toBe(false); }); @@ -220,14 +232,15 @@ describe(`schedules`, function () { expect(ScheduleDataValidator.validate(scheduleForServiceId, pathStub1).isValid).toBe(true); expect(ScheduleDataValidator.validate(scheduleForServiceId, pathStub2).isValid).toBe(false); const newId = await dbQueries.save(scheduleForServiceId as any); - expect(newId).toBe(scheduleForServiceId.id); - + expect(newId).not.toBe(scheduleForServiceId.integer_id); + scheduleIntegerId = newId; }); test('should not create schedule object with existing service/line pair', async function() { const existingServiceLineSchedule = _cloneDeep(scheduleForServiceId); existingServiceLineSchedule.id = uuidV4(); + existingServiceLineSchedule.integer_id = undefined; existingServiceLineSchedule.periods = []; let exception: any = undefined; try { @@ -242,30 +255,41 @@ describe(`schedules`, function () { test('schedule exists should return true if object is in database', async function () { // Check unexisting schedule - const exists = await dbQueries.exists(scheduleForServiceId.id); + const exists = await dbQueries.exists(scheduleIntegerId as number); expect(exists).toBe(true); }); test('should read schedule object as schedule data with periods and trips', async function() { - const scheduleDataRead = await dbQueries.read(scheduleForServiceId.id); - expect(scheduleDataRead).toMatchObject(scheduleForServiceId); + const scheduleDataRead = await dbQueries.read(scheduleIntegerId as number); + expectSchedulesSame(scheduleDataRead, scheduleForServiceId as any); expect(scheduleDataRead.updated_at).toBeNull(); expect(scheduleDataRead.created_at).not.toBeNull(); }); + test('readForLine', async() => { + // Read the schedule for the line ID requested + const schedulesForLine = await dbQueries.readForLine(scheduleForServiceId.line_id); + expect(schedulesForLine.length).toEqual(1); + expectSchedulesSame(schedulesForLine[0], scheduleForServiceId as any); + + // Read for a line id without data + const schedulesForNonexistentLine = await dbQueries.readForLine(uuidV4()); + expect(schedulesForNonexistentLine).toEqual([]); + }); + test('test collection', async function() { const collection = await dbQueries.collection(); expect(collection.length).toEqual(1); - expect(collection[0]).toMatchObject(scheduleForServiceId); + expectSchedulesSame(collection[0], scheduleForServiceId as any); }) test('should update a schedule in database and read it correctly', async () => { // Read the object from DB to get all the IDs - const scheduleDataRead = await dbQueries.read(scheduleForServiceId.id); + const scheduleDataRead = await dbQueries.read(scheduleIntegerId as number); // Change a few values in schedule and 2nd period and trip const updatedSchedule = _cloneDeep(scheduleDataRead); @@ -274,8 +298,8 @@ describe(`schedules`, function () { updatedSchedule.periods[1].trips[0].seated_capacity = 30; // Update the object - const newId = await dbQueries.save(updatedSchedule); - expect(newId).toBe(scheduleForServiceId.id); + const updatedId = await dbQueries.save(updatedSchedule); + expect(updatedId).toBe(scheduleIntegerId); // Delete the updated_at fields updatedSchedule.updated_at = expect.anything(); @@ -285,7 +309,7 @@ describe(`schedules`, function () { }); // Read the object again and make sure it matches - const scheduleDataUpdatedRead = await dbQueries.read(scheduleForServiceId.id); + const scheduleDataUpdatedRead = await dbQueries.read(scheduleIntegerId as number); expectSchedulesSame(scheduleDataUpdatedRead, updatedSchedule); expect(scheduleDataUpdatedRead.updated_at).not.toBeNull(); expect(scheduleDataUpdatedRead.created_at).not.toBeNull(); @@ -295,7 +319,7 @@ describe(`schedules`, function () { test('Update a schedule after deleting trips and periods', async () => { // Read the object from DB to get all the IDs - const scheduleDataRead = await dbQueries.read(scheduleForServiceId.id); + const scheduleDataRead = await dbQueries.read(scheduleIntegerId as number); // Remove 2nd period and a trip from first period const updatedSchedule = _cloneDeep(scheduleDataRead); @@ -303,8 +327,8 @@ describe(`schedules`, function () { updatedSchedule.periods[0].trips.splice(2, 1); // Update the object - const newId = await dbQueries.save(updatedSchedule); - expect(newId).toBe(scheduleForServiceId.id); + const updatedId = await dbQueries.save(updatedSchedule); + expect(updatedId).toBe(scheduleIntegerId); // Expect anything for the updated_at fields updatedSchedule.updated_at = expect.anything(); @@ -314,7 +338,7 @@ describe(`schedules`, function () { }); // Read the object again and make sure it matches - const scheduleDataUpdatedRead = await dbQueries.read(scheduleForServiceId.id); + const scheduleDataUpdatedRead = await dbQueries.read(scheduleIntegerId as number); // Recursively remove the updated_at field scheduleDataUpdatedRead.periods.forEach((period) => { delete period.updated_at; @@ -326,7 +350,7 @@ describe(`schedules`, function () { test('Update a schedule after adding trips and periods', async () => { // Read the object from DB to get all the IDs - const scheduleDataRead = await dbQueries.read(scheduleForServiceId.id); + const scheduleDataRead = await dbQueries.read(scheduleIntegerId as number); // Remove 2nd period and a trip from first period const updatedSchedule = _cloneDeep(scheduleDataRead); @@ -336,8 +360,8 @@ describe(`schedules`, function () { updatedSchedule.periods[0].trips.push(newTrip as any); // Update the object - const newId = await dbQueries.save(updatedSchedule); - expect(newId).toBe(scheduleForServiceId.id); + const updatedId = await dbQueries.save(updatedSchedule); + expect(updatedId).toBe(scheduleIntegerId); // Expect anything for the updated_at fields updatedSchedule.updated_at = expect.anything(); @@ -347,18 +371,34 @@ describe(`schedules`, function () { }); // Read the object again and make sure it matches - const scheduleDataUpdatedRead = await dbQueries.read(scheduleForServiceId.id); + const scheduleDataUpdatedRead = await dbQueries.read(scheduleIntegerId as number); expectSchedulesSame(scheduleDataUpdatedRead, updatedSchedule); }); test('should delete object from database', async () => { - const id = await dbQueries.delete(scheduleForServiceId.id); - expect(id).toBe(scheduleForServiceId.id); + const id = await dbQueries.delete(scheduleIntegerId as number); + expect(id).toBe(scheduleIntegerId); // Verify the object does not exist anymore - const exists = await dbQueries.exists(scheduleForServiceId.id); + const exists = await dbQueries.exists(scheduleIntegerId as number); + expect(exists).toBe(false); + + }); + + test('should delete object from database by uuid', async () => { + // FIXME We should not support deletion by uuid, remove when it is not supported anymore + // Insert the new object in the DB + const newId = await dbQueries.save(scheduleForServiceId as any); + const existsBefore = await dbQueries.exists(newId as number); + expect(existsBefore).toBe(true); + + // Delete by uuid + await dbQueries.delete(scheduleForServiceId.id); + + // Verify the object does not exist anymore + const exists = await dbQueries.exists(scheduleIntegerId as number); expect(exists).toBe(false); }); @@ -382,15 +422,15 @@ describe('Schedules, single queries with transaction errors', () => { await expect(dbQueries.save(newSchedule as any)).rejects.toThrowError(TrError); // Read the object from DB and make sure it has not changed - const dataExists = await dbQueries.exists(scheduleForServiceId.id); + const dataExists = await dbQueries.exists(scheduleIntegerId as number); expect(dataExists).toEqual(false); }); test('update with periods and trips, with error', async() => { // Insert the schedule - await dbQueries.save(scheduleForServiceId as any); + const newId = await dbQueries.save(scheduleForServiceId as any); // Read the object from DB and make sure it has not changed - const originalData = await dbQueries.read(scheduleForServiceId.id); + const originalData = await dbQueries.read(newId); // Force and invalid data for period field custom_start_at_str const updatedSchedule = _cloneDeep(scheduleForServiceId); @@ -400,7 +440,7 @@ describe('Schedules, single queries with transaction errors', () => { await expect(dbQueries.save(updatedSchedule as any)).rejects.toThrowError(TrError); // Read the object from DB and make sure it has not changed - const dataAfterFail = await dbQueries.read(scheduleForServiceId.id); + const dataAfterFail = await dbQueries.read(newId); expectSchedulesSame(dataAfterFail, originalData); }); @@ -418,37 +458,59 @@ describe('Schedules, with transactions', () => { test('Create, update with success', async() => { const originalSchedule = _cloneDeep(scheduleForServiceId) as any; - const updatedSchedule = _cloneDeep(originalSchedule); - // Remove 2nd period and a trip from first period, then - updatedSchedule.periods.splice(1, 1); - updatedSchedule.periods[0].trips.splice(2, 1); + let originalUpdatedSchedule: any = undefined; + let newId: any = undefined; await knex.transaction(async (trx) => { // Save the original schedule - await dbQueries.save(originalSchedule, { transaction: trx }); - - // Save the updated schedule with one less period and trip + newId = await dbQueries.save(originalSchedule, { transaction: trx }); + + // Read the schedule, then save the updated schedule with one less period and trip + const updatedSchedule = await dbQueries.read(newId, { transaction: trx }); + delete updatedSchedule.updated_at; + updatedSchedule.periods.forEach((period) => { + delete period.updated_at; + if (period.trips) { + period.trips.forEach((trip) => delete trip.updated_at); + } + }) + // Remove 2nd period and a trip from first period, then + updatedSchedule.periods.splice(1, 1); + updatedSchedule.periods[0].trips.splice(2, 1); + originalUpdatedSchedule = updatedSchedule; await dbQueries.save(updatedSchedule, { transaction: trx }); }); // Make sure the object is there and updated - const dataRead = await dbQueries.read(scheduleForServiceId.id); - expectSchedulesSame(dataRead, updatedSchedule); + const dataRead = await dbQueries.read(newId); + expectSchedulesSame(dataRead, originalUpdatedSchedule); }); test('Create, update with error', async() => { const originalSchedule = _cloneDeep(scheduleForServiceId) as any; - const updatedSchedule = _cloneDeep(originalSchedule); - // Update some fields, but change uuid of one trip for not a uuid - updatedSchedule.allow_seconds_based_schedules = true; - updatedSchedule.periods.splice(1, 1); - updatedSchedule.periods[0].trips[0].id = 'not a uuid'; + let originalUpdatedSchedule: any = undefined; + let newId: any = undefined; let error: any = undefined; try { await knex.transaction(async (trx) => { // Save the original schedule - await dbQueries.save(originalSchedule, { transaction: trx }); + newId = await dbQueries.save(originalSchedule, { transaction: trx }); + + // Read the schedule, then save the updated schedule with one less period and trip + const updatedSchedule = await dbQueries.read(newId, { transaction: trx }); + delete updatedSchedule.updated_at; + updatedSchedule.periods.forEach((period) => { + delete period.updated_at; + if (period.trips) { + period.trips.forEach((trip) => delete trip.updated_at); + } + }) + // Update some fields, but change uuid of one trip for not a uuid + updatedSchedule.allow_seconds_based_schedules = true; + updatedSchedule.periods.splice(1, 1); + updatedSchedule.periods[0].trips[0].id = 'not a uuid'; + originalUpdatedSchedule = updatedSchedule; // Save the updated schedule with one less period and trip await dbQueries.save(updatedSchedule, { transaction: trx }); @@ -459,16 +521,16 @@ describe('Schedules, with transactions', () => { expect(error).toBeDefined(); // Read the object from DB and make sure it has not changed - const dataExists = await dbQueries.exists(scheduleForServiceId.id); + const dataExists = await dbQueries.exists(newId); expect(dataExists).toEqual(false); }); test('Update, delete with error', async() => { const originalSchedule = _cloneDeep(scheduleForServiceId) as any; // Add the original schedule out of the transaction - await dbQueries.save(originalSchedule); + const newId = await dbQueries.save(originalSchedule); - const updatedSchedule = _cloneDeep(originalSchedule); + const updatedSchedule = await dbQueries.read(newId); // Remove 2nd period and a trip from first period, then updatedSchedule.periods.splice(1, 1); updatedSchedule.periods[0].trips.splice(2, 1); @@ -478,7 +540,8 @@ describe('Schedules, with transactions', () => { await knex.transaction(async (trx) => { // Update, then delete the schedule, then throw an error await dbQueries.save(updatedSchedule, { transaction: trx }); - await dbQueries.delete(scheduleForServiceId.id, { transaction: trx }); + + await dbQueries.delete(newId, { transaction: trx }); throw 'error'; }); } catch(err) { @@ -488,7 +551,7 @@ describe('Schedules, with transactions', () => { // Make sure the original object is unchanged // Make sure the object is there and updated - const dataRead = await dbQueries.read(scheduleForServiceId.id); + const dataRead = await dbQueries.read(newId); expectSchedulesSame(dataRead, originalSchedule); }); diff --git a/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts b/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts index c0d162a87..74ffc2965 100644 --- a/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts +++ b/packages/transition-backend/src/models/db/__tests__/transitLines.db.test.ts @@ -221,8 +221,10 @@ describe(`${objectName}`, () => { const _updatedAttributes = Object.assign({}, newObjectAttributesWithSchedule); const updatedObject = new ObjectClass(_updatedAttributes, false); const id = await dbQueries.update(newObjectAttributesWithSchedule.id, updatedObject.getAttributes()); - await schedulesDbQueries.save(scheduleForServiceId); - await schedulesDbQueries.save(scheduleForServiceId2); + const schedId1 = await schedulesDbQueries.save(scheduleForServiceId); + (scheduleForServiceId as any).integer_id = schedId1; + const schedId2 = await schedulesDbQueries.save(scheduleForServiceId2); + (scheduleForServiceId2 as any).integer_id = schedId2; await pathsDbQueries.create(pathAttributes); expect(id).toBe(newObjectAttributesWithSchedule.id); diff --git a/packages/transition-backend/src/models/db/migrations/20241003145700_useNumericIdsInSchedulesTables.ts b/packages/transition-backend/src/models/db/migrations/20241003145700_useNumericIdsInSchedulesTables.ts new file mode 100644 index 000000000..87c9e0bb4 --- /dev/null +++ b/packages/transition-backend/src/models/db/migrations/20241003145700_useNumericIdsInSchedulesTables.ts @@ -0,0 +1,158 @@ +/* + * Copyright 2024, Polytechnique Montreal and contributors + * + * This file is licensed under the MIT License. + * License text available at https://opensource.org/licenses/MIT + */ +import { Knex } from 'knex'; + +const scheduleTblName = 'tr_transit_schedules'; +const schedulePeriodTblName = 'tr_transit_schedule_periods'; +const scheduleTripTblName = 'tr_transit_schedule_trips'; + +/** + * This migration changes the primary key column of the 3 tables describing + * schedules to be a numeric auto-increment ID. It is not necessary to have a + * uuid and numeric auto-increment IDs will allow more performant duplication + * operations, among other things. + * + * We keep the uuid columns, but it will not be used as primary or foreign keys + * anymore, just for json2capnp conversion. + * + * After this migration, the currently named *_id columns in the 3 schedule + * tables will now be integers instead of uuid, the trip does not force a link + * to the schedule anymore as we have this link through the period. + * + * @param knex The database configuration object + * @returns + */ +export async function up(knex: Knex): Promise { + // Rename the current uuid columns, and drop the foreign key constraints for now + await knex.schema.alterTable(scheduleTblName, (table: Knex.TableBuilder) => { + table.renameColumn('id', 'uuid'); + }); + await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.renameColumn('id', 'uuid'); + table.renameColumn('schedule_id', 'schedule_uuid'); + table.dropForeign('schedule_id'); + }); + await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.renameColumn('id', 'uuid'); + table.renameColumn('schedule_id', 'schedule_uuid'); + table.renameColumn('schedule_period_id', 'schedule_period_uuid'); + table.dropForeign('schedule_id'); + table.dropForeign('schedule_period_id'); + }); + + // Drop all primary keys from table, to better recreate them + await knex.schema.alterTable(scheduleTblName, (table: Knex.TableBuilder) => { + table.dropPrimary(); + }); + await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.dropPrimary(); + }); + await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.dropPrimary(); + }); + + // Add the new auto-incrementing primary key id columns and columns for + // foreign key with nullable values + await knex.schema.alterTable(scheduleTblName, (table: Knex.TableBuilder) => { + table.increments(); + }); + await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.increments(); + table.integer('schedule_id').nullable(); + }); + await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.increments(); + table.integer('schedule_period_id').nullable(); + }); + + // Set the values for the foreign key columns + await knex.raw(` + UPDATE ${schedulePeriodTblName} sp + SET schedule_id = s.id + FROM ${scheduleTblName} s + WHERE sp.schedule_uuid = s.uuid; + `); + await knex.raw(` + UPDATE ${scheduleTripTblName} st + SET schedule_period_id = sp.id + FROM ${schedulePeriodTblName} sp + WHERE st.schedule_period_uuid = sp.uuid; + `); + + // Drop the previous foreign uuid key columns, make the new foreign key + // columns not nullable and add foreign key constraints + await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.dropColumns('schedule_uuid', 'schedule_period_uuid'); + table.integer('schedule_period_id').notNullable().index().alter(); + table.foreign('schedule_period_id').references(`${schedulePeriodTblName}.id`).onDelete('CASCADE'); + }); + return await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.dropColumns('schedule_uuid'); + table.integer('schedule_id').notNullable().index().alter(); + table.foreign('schedule_id').references(`${scheduleTblName}.id`).onDelete('CASCADE'); + }); +} + +export async function down(knex: Knex): Promise { + // Create the uuid foreign key columns in tables + await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.uuid('schedule_uuid').nullable(); + }); + await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.uuid('schedule_uuid').nullable(); + table.uuid('schedule_period_uuid').nullable(); + }); + + // Set the values of the uuid columns from current id columns + await knex.raw(` + UPDATE ${schedulePeriodTblName} sp + SET schedule_uuid = s.uuid + FROM ${scheduleTblName} s + WHERE sp.schedule_id = s.id; + `); + await knex.raw(` + UPDATE ${scheduleTripTblName} st + SET schedule_period_uuid = sp.uuid, schedule_uuid = s.uuid + FROM ${schedulePeriodTblName} sp + INNER JOIN ${scheduleTblName} s ON sp.schedule_id = s.id + WHERE st.schedule_period_id = sp.id; + `); + + // Delete the id columns, set uuids as primary keys and make foreign key columns not nullable and indexed + await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.dropColumns('id', 'schedule_period_id'); + table.uuid('uuid').primary().alter(); + table.uuid('schedule_period_uuid').notNullable().index().alter(); + table.uuid('schedule_uuid').notNullable().index().alter(); + }); + await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.dropColumns('id', 'schedule_id'); + table.uuid('uuid').primary().alter(); + table.uuid('schedule_uuid').notNullable().index().alter(); + }); + await knex.schema.alterTable(scheduleTblName, (table: Knex.TableBuilder) => { + table.dropColumn('id'); + table.uuid('uuid').primary().alter(); + }); + + // Revert name change and add foreign key constraints + await knex.schema.alterTable(scheduleTblName, (table: Knex.TableBuilder) => { + table.renameColumn('uuid', 'id'); + }); + await knex.schema.alterTable(schedulePeriodTblName, (table: Knex.TableBuilder) => { + table.renameColumn('uuid', 'id'); + table.renameColumn('schedule_uuid', 'schedule_id'); + table.foreign('schedule_id').references('tr_transit_schedules.id').onDelete('CASCADE'); + }); + return await knex.schema.alterTable(scheduleTripTblName, (table: Knex.TableBuilder) => { + table.renameColumn('uuid', 'id'); + table.renameColumn('schedule_uuid', 'schedule_id'); + table.renameColumn('schedule_period_uuid', 'schedule_period_id'); + table.foreign('schedule_id').references('tr_transit_schedules.id').onDelete('CASCADE'); + table.foreign('schedule_period_id').references('tr_transit_schedule_periods.id').onDelete('CASCADE'); + }); +} diff --git a/packages/transition-backend/src/models/db/transitPaths.db.queries.ts b/packages/transition-backend/src/models/db/transitPaths.db.queries.ts index b607db12d..a5e988646 100644 --- a/packages/transition-backend/src/models/db/transitPaths.db.queries.ts +++ b/packages/transition-backend/src/models/db/transitPaths.db.queries.ts @@ -28,6 +28,7 @@ import { PathAttributes } from 'transition-common/lib/services/path/Path'; const tableName = 'tr_transit_paths'; const linesTableName = 'tr_transit_lines'; const schedulesTableName = 'tr_transit_schedules'; +const periodsTableName = 'tr_transit_schedule_periods'; const tripsTableName = 'tr_transit_schedule_trips'; const scenariosServicesTableName = 'tr_transit_scenario_services'; const st = knexPostgis(knex); @@ -162,7 +163,8 @@ const geojsonCollection = async ( if (params.scenarioId) { baseQuery .innerJoin(`${tripsTableName} as trips`, 'trips.path_id', 'p.id') - .innerJoin(`${schedulesTableName} as sched`, 'sched.id', 'trips.schedule_id') + .innerJoin(`${periodsTableName} as periods`, 'periods.id', 'trips.schedule_period_id') + .innerJoin(`${schedulesTableName} as sched`, 'sched.id', 'periods.schedule_id') .innerJoin(`${scenariosServicesTableName} as sc`, 'sched.service_id', 'sc.service_id') .andWhere('sc.scenario_id', params.scenarioId) .groupBy('p.id', 'l.color', 'l.mode'); @@ -179,7 +181,8 @@ const geojsonCollectionForServices = async ( const baseQuery = getGeojsonBaseQuery(true); baseQuery .innerJoin(`${tripsTableName} as trips`, 'trips.path_id', 'p.id') - .innerJoin(`${schedulesTableName} as sched`, 'sched.id', 'trips.schedule_id') + .innerJoin(`${periodsTableName} as periods`, 'periods.id', 'trips.schedule_period_id') + .innerJoin(`${schedulesTableName} as sched`, 'sched.id', 'periods.schedule_id') .whereIn('sched.service_id', serviceIds) .groupBy('p.id', 'l.color', 'l.mode'); return await geojsonCollectionFromQuery(baseQuery); diff --git a/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts b/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts index f91464681..17b92767a 100644 --- a/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts +++ b/packages/transition-backend/src/models/db/transitSchedules.db.queries.ts @@ -36,15 +36,34 @@ const scheduleTable = 'tr_transit_schedules'; const periodTable = 'tr_transit_schedule_periods'; const tripTable = 'tr_transit_schedule_trips'; +// In the database, the schedule table has a uuid column, and numeric id, but this needs to be mapped respectively to the string id and integer_id + const scheduleAttributesCleaner = function (attributes: Partial): Partial { - const _attributes = _cloneDeep(attributes); + const { integer_id, id, ...rest } = attributes; + const _attributes = _cloneDeep(rest) as any; delete _attributes.periods; + if (typeof integer_id === 'number' && integer_id > 0) { + _attributes.id = integer_id; + } else { + delete _attributes.id; + } + _attributes.uuid = id; return _attributes; }; +const scheduleAttributesParser = function (attributes: { [key: string]: any }): Partial { + const { id, uuid, ...rest } = attributes; + return { + ...rest, + id: uuid, + integer_id: id + }; +}; + const schedulePeriodsAttributesCleaner = function (attributes: Partial): { [key: string]: any } { const { id, + integer_id, schedule_id, outbound_path_id, inbound_path_id, @@ -57,7 +76,8 @@ const schedulePeriodsAttributesCleaner = function (attributes: Partial 0 ? integer_id : undefined, schedule_id, outbound_path_id, inbound_path_id, @@ -72,10 +92,19 @@ const schedulePeriodsAttributesCleaner = function (attributes: Partial= 0 ? period_start_at_seconds / 3600 : null, end_at_hour: period_end_at_seconds >= 0 ? period_end_at_seconds / 3600 : null, custom_start_at_str: @@ -85,7 +114,10 @@ const schedulePeriodsAttributesParser = function (attributes: any): SchedulePeri }; const scheduleTripsAttributesCleaner = function (attributes: Partial): { [key: string]: any } { - const _attributes = _cloneDeep(attributes) as any; + const { id, integer_id, ...rest } = attributes; + const _attributes = _cloneDeep(rest) as any; + _attributes.uuid = id || uuidV4(); + _attributes.id = typeof integer_id === 'number' && integer_id > 0 ? integer_id : undefined; delete _attributes.unitReadyAt; delete _attributes.unitDirection; delete _attributes.is_frozen; @@ -94,13 +126,18 @@ const scheduleTripsAttributesCleaner = function (attributes: Partial 0 ? attributes.integer_id : undefined; return _attributes; }; const scheduleTripsAttributesParser = function (attributes: any): SchedulePeriodTrip { - const { node_departure_time_seconds, node_arrival_time_seconds, ...rest } = attributes; + const { id, uuid, node_departure_time_seconds, node_arrival_time_seconds, ...rest } = attributes; return { ...rest, + id: uuid, + integer_id: id, node_departure_times_seconds: node_departure_time_seconds, node_arrival_times_seconds: node_arrival_time_seconds }; @@ -112,7 +149,7 @@ const _createTrips = async function ( options: { transaction: Knex.Transaction } ): Promise<{ id: string }[]> { const ids = (await createMultiple(knex, tripTable, scheduleTripsAttributesCleaner, periodTrips, { - returning: 'id', + returning: 'uuid', transaction: options.transaction })) as { id: string }[]; return ids; @@ -121,7 +158,7 @@ const _createTrips = async function ( // Private function to insert periods, with a transaction const _createPeriods = async function ( schedulePeriods: SchedulePeriod[], - scheduleId: string, + scheduleId: number, options: { transaction: Knex.Transaction } ): Promise<{ id: string }[]> { schedulePeriods.forEach((period) => { @@ -132,9 +169,14 @@ const _createPeriods = async function ( }); const ids = (await createMultiple(knex, periodTable, schedulePeriodsAttributesCleaner, schedulePeriods, { - returning: 'id', + returning: ['id', 'uuid'], transaction: options.transaction - })) as { id: string }[]; + })) as { id: number; uuid: string }[]; + // Update period's ids + schedulePeriods.forEach((period, idx) => { + period.id = ids[idx].uuid; + period.integer_id = ids[idx].id; + }); const tripPromises = schedulePeriods .filter((period) => period.trips !== undefined) .map((period) => { @@ -142,13 +184,12 @@ const _createPeriods = async function ( if (!trip.id) { trip.id = uuidV4(); } - trip.schedule_id = scheduleId; - trip.schedule_period_id = period.id; + trip.schedule_period_id = period.integer_id as number; }); return _createTrips(period.trips, options); }); await Promise.all(tripPromises); - return ids; + return ids.map((idUuid) => ({ id: idUuid.uuid })); }; /** @@ -162,18 +203,18 @@ const _createPeriods = async function ( const createFromScheduleData = async ( scheduleData: ScheduleAttributes, { transaction }: { transaction?: Knex.Transaction } = {} -): Promise => { +): Promise => { try { // Nested function to require a transaction around the insert const createWithTransaction = async (trx: Knex.Transaction) => { - const id = (await create(knex, scheduleTable, scheduleAttributesCleaner, scheduleData, { - returning: 'id', + const idUuid = (await create(knex, scheduleTable, scheduleAttributesCleaner, scheduleData, { + returning: ['id', 'uuid'], transaction: trx - })) as string; + })) as { id: number; uuid: string }; if (scheduleData.periods) { - await _createPeriods(scheduleData.periods, id, { transaction: trx }); + await _createPeriods(scheduleData.periods, idUuid.id, { transaction: trx }); } - return id; + return idUuid.id; }; // Make sure the insert is done in a transaction, use the one in argument if available return await (transaction ? createWithTransaction(transaction) : knex.transaction(createWithTransaction)); @@ -192,7 +233,7 @@ const _updateTrips = async function ( options: { transaction: Knex.Transaction } ) { periodTrips.forEach((periodTrip) => { - if (!periodTrip.id || !periodTrip.schedule_id || !periodTrip.schedule_period_id) { + if (!periodTrip.id || !periodTrip.schedule_period_id) { throw 'Missing trip, schedule or period id for trip, cannot update'; } }); @@ -209,7 +250,6 @@ const _updateTrips = async function ( // which operation needs to be executed for each trip. const _updateTripsForPeriods = async ( schedulePeriods: Partial[], - scheduleId: string, options: { transaction: Knex.Transaction } ) => { // Trips need to be inserted, updated or deleted @@ -217,7 +257,7 @@ const _updateTripsForPeriods = async ( // Get the previous trips for those periods const previousTripIds = await _getTripIdsForPeriods( - schedulePeriods.map((period) => period.id as string), + schedulePeriods.map((period) => period.integer_id as number), options ); const currentTripIds: string[] = []; @@ -235,8 +275,7 @@ const _updateTripsForPeriods = async ( tripsToUpdate.push(trip); } else { // Add to trips to insert - trip.schedule_id = scheduleId; - trip.schedule_period_id = period.id as string; + trip.schedule_period_id = period.integer_id as number; tripsToInsert.push(trip); } } @@ -259,7 +298,6 @@ const _updateTripsForPeriods = async ( // Private function to update periods, with transaction const _updatePeriods = async ( schedulePeriods: Partial[], - scheduleId: string, options: { transaction: Knex.Transaction } ) => { schedulePeriods.forEach((schedulePeriod) => { @@ -269,13 +307,13 @@ const _updatePeriods = async ( }); const ids = await updateMultiple(knex, periodTable, schedulePeriodsAttributesCleaner, schedulePeriods, { - returning: 'id', + returning: 'uuid', transaction: options.transaction }); const periodsWithTrips = schedulePeriods.filter((period) => period.trips !== undefined); if (periodsWithTrips.length > 0) { - await _updateTripsForPeriods(periodsWithTrips, scheduleId, options); + await _updateTripsForPeriods(periodsWithTrips, options); } return ids; }; @@ -291,10 +329,10 @@ const _updatePeriods = async ( * @returns The ID of the updated schedule */ const updateFromScheduleData = async ( - scheduleId: string, + scheduleId: number, scheduleData: ScheduleAttributes, { transaction }: { transaction?: Knex.Transaction } = {} -): Promise => { +): Promise => { try { // Nested function to require a transaction around the update const updateWithTransaction = async (trx: Knex.Transaction) => { @@ -302,7 +340,7 @@ const updateFromScheduleData = async ( const id = (await update(knex, scheduleTable, scheduleAttributesCleaner, scheduleId, scheduleData, { returning: 'id', transaction: trx - })) as string; + })) as number; // Quick return if there are no periods to update if (scheduleData.periods === undefined) { @@ -312,16 +350,16 @@ const updateFromScheduleData = async ( // Periods need to be inserted, updated or deleted const createUpdateDeletePromises: Promise[] = []; - const previousPeriodIds = await _getPeriodIdsForSchedule(id, { transaction: trx }); - const currentPeriods: string[] = []; + const previousPeriodIds = await _getPeriodIdsForSchedule(scheduleId, { transaction: trx }); + const currentPeriodIds: number[] = []; const periodsToInsert: SchedulePeriod[] = []; const periodsToUpdate: SchedulePeriod[] = []; // Update or create periods in schedule for (let i = 0; i < scheduleData.periods.length; i++) { const period = scheduleData.periods[i]; - if (previousPeriodIds.includes(period.id)) { + if (period.integer_id && previousPeriodIds.includes(period.integer_id)) { // Add to current periods and to periods to update - currentPeriods.push(period.id); + currentPeriodIds.push(period.integer_id); periodsToUpdate.push(period); } else { // Add to periods to insert @@ -332,13 +370,13 @@ const updateFromScheduleData = async ( createUpdateDeletePromises.push(_createPeriods(periodsToInsert, scheduleId, { transaction: trx })); } if (periodsToUpdate.length > 0) { - createUpdateDeletePromises.push(_updatePeriods(periodsToUpdate, scheduleId, { transaction: trx })); + createUpdateDeletePromises.push(_updatePeriods(periodsToUpdate, { transaction: trx })); } // Delete any period that was deleted - const deletedPeriods = previousPeriodIds.filter((periodId) => !currentPeriods.includes(periodId)); - if (deletedPeriods.length > 0) { - createUpdateDeletePromises.push(_deleteSchedulePeriods(deletedPeriods, { transaction: trx })); + const deletedPeriodIds = previousPeriodIds.filter((periodId) => !currentPeriodIds.includes(periodId)); + if (deletedPeriodIds.length > 0) { + createUpdateDeletePromises.push(_deleteSchedulePeriods(deletedPeriodIds, { transaction: trx })); } await Promise.all(createUpdateDeletePromises); @@ -356,18 +394,20 @@ const updateFromScheduleData = async ( }; const save = async function (scheduleData: ScheduleAttributes, options: { transaction?: Knex.Transaction } = {}) { - const scheduleExists = await exists(knex, scheduleTable, scheduleData.id, { transaction: options.transaction }); + const scheduleExists = scheduleData.integer_id + ? await exists(knex, scheduleTable, scheduleData.integer_id, { transaction: options.transaction }) + : false; return scheduleExists - ? await updateFromScheduleData(scheduleData.id, scheduleData, options) + ? await updateFromScheduleData(scheduleData.integer_id as number, scheduleData, options) : await createFromScheduleData(scheduleData, options); }; // Private function to get the period ids for a given schedule, select within // the current transaction const _getPeriodIdsForSchedule = async function ( - schedule_id: string, + schedule_id: number, options: { transaction: Knex.Transaction } -): Promise { +): Promise { const periodQueries = knex(periodTable) .select('id') .where('schedule_id', schedule_id) @@ -379,7 +419,7 @@ const _getPeriodIdsForSchedule = async function ( // Private function to get the trip ids for the given periods, select within the // current transaction -const _getTripIdsForPeriods = async function (periodIds: string[], options: { transaction: Knex.Transaction }) { +const _getTripIdsForPeriods = async function (periodIds: number[], options: { transaction: Knex.Transaction }) { const trips = await knex(tripTable) .select('id') .whereIn('schedule_period_id', periodIds) @@ -393,7 +433,7 @@ const getScheduleIdsForLine = async function ( transaction?: Knex.Transaction; } = {} ) { - const scheduleQueries = knex(scheduleTable).select(knex.raw('id')).where('line_id', line_id); + const scheduleQueries = knex(scheduleTable).select('id').where('line_id', line_id); if (options.transaction) { scheduleQueries.transacting(options.transaction); } @@ -401,8 +441,12 @@ const getScheduleIdsForLine = async function ( return rows.map((row) => (row as any).id); }; -const readScheduleTrips = async function (period_id: string) { - const rows = await knex(tripTable).select(knex.raw('*')).where('schedule_period_id', period_id); +const readScheduleTrips = async function (period_id: number, options: { transaction?: Knex.Transaction } = {}) { + const query = knex(tripTable).select(knex.raw('*')).where('schedule_period_id', period_id); + if (options.transaction) { + query.transacting(options.transaction); + } + const rows = await query; const trips: SchedulePeriodTrip[] = []; for (let i = 0; i < rows.length; i++) { trips.push(scheduleTripsAttributesParser(rows[i])); @@ -410,21 +454,27 @@ const readScheduleTrips = async function (period_id: string) { return trips; }; -const readSchedulePeriods = async function (id: string) { - const rows = await knex(periodTable).select(knex.raw('*')).where('schedule_id', id); +const readSchedulePeriods = async function (id: number, options: { transaction?: Knex.Transaction } = {}) { + const query = knex(periodTable).select(knex.raw('*')).where('schedule_id', id); + if (options.transaction) { + query.transacting(options.transaction); + } + const rows = await query; const periods: SchedulePeriod[] = []; for (let i = 0; i < rows.length; i++) { const period = schedulePeriodsAttributesParser(rows[i]); periods.push(period); - period.trips = await readScheduleTrips(period.id); + period.trips = await readScheduleTrips(period.integer_id!, options); period.trips.sort((a, b) => a.departure_time_seconds - b.departure_time_seconds); } return periods; }; -const readScheduleData = async function (id: string) { - const schedule = (await read(knex, scheduleTable, undefined, '*', id)) as ScheduleAttributes; - schedule.periods = await readSchedulePeriods(id); +const readScheduleData = async function (id: number, options: { transaction?: Knex.Transaction } = {}) { + const schedule = scheduleAttributesParser( + await read(knex, scheduleTable, undefined, '*', id, options) + ) as ScheduleAttributes; + schedule.periods = await readSchedulePeriods(id, options); schedule.periods.sort((a, b) => a.start_at_hour - b.start_at_hour); return schedule; }; @@ -432,20 +482,28 @@ const readScheduleData = async function (id: string) { const readForLine = async function (lineId: string) { // TODO Read objects independently. It would be possible to make it all in one query to DB if performance becomes problematic const scheduleIds = await getScheduleIdsForLine(lineId); - return await Promise.all(scheduleIds.map(readScheduleData)); + return await Promise.all(scheduleIds.map((scheduleId) => readScheduleData(scheduleId))); }; // Private function to delete periods by ids, within a transaction -const _deleteSchedulePeriods = async function (ids: string[], options: { transaction: Knex.Transaction }) { +const _deleteSchedulePeriods = async function (ids: number[], options: { transaction: Knex.Transaction }) { return await deleteMultiple(knex, periodTable, ids, options); }; // Private function to delete trips by id, within a transaction -const _deleteSchedulePeriodTrips = async function (ids: string[], options: { transaction: Knex.Transaction }) { +const _deleteSchedulePeriodTrips = async function (ids: number[], options: { transaction: Knex.Transaction }) { return await deleteMultiple(knex, tripTable, ids, options); }; -const deleteScheduleData = async function (id: string, options: Parameters[3] = {}) { +const deleteScheduleData = async function (id: number | string, options: Parameters[3] = {}) { + // FIXME The main workflow still receives the uuid of the schedule to delete instead of the numeric id, so have to handle both + if (typeof id === 'string') { + const query = knex(scheduleTable).where('uuid', id).del(); + if (options.transaction) { + query.transacting(options.transaction); + } + return await query; + } return await deleteRecord(knex, scheduleTable, id, options); }; @@ -477,7 +535,7 @@ const getCollectionSubquery = () => { }; const dbRowToScheduleAttributes = (row: any): ScheduleAttributes => { - const schedule = row as unknown as ScheduleAttributes; + const schedule = scheduleAttributesParser(row) as ScheduleAttributes; // Clean attributes if (schedule.periods && schedule.periods[0]) { schedule.periods = schedule.periods.map((period) => { diff --git a/packages/transition-backend/src/services/evolutionaryAlgorithm/preparation/ServicePreparation.ts b/packages/transition-backend/src/services/evolutionaryAlgorithm/preparation/ServicePreparation.ts index 74a57c305..e0e69832e 100644 --- a/packages/transition-backend/src/services/evolutionaryAlgorithm/preparation/ServicePreparation.ts +++ b/packages/transition-backend/src/services/evolutionaryAlgorithm/preparation/ServicePreparation.ts @@ -104,7 +104,7 @@ const prepareServicesForLines = async ( schedule.attributes.periods.push( Object.assign({}, periodAttributes, { id: uuidV4(), - schedule_id: schedule.getId(), + schedule_id: schedule.attributes.integer_id || -1, outbound_path_id: outboundPathId, inbound_path_id: inboundPathId, number_of_units: nbVehicles, diff --git a/packages/transition-backend/src/services/gtfsExport/__tests__/ScheduleExporter.test.ts b/packages/transition-backend/src/services/gtfsExport/__tests__/ScheduleExporter.test.ts index 12e437205..f992e858c 100644 --- a/packages/transition-backend/src/services/gtfsExport/__tests__/ScheduleExporter.test.ts +++ b/packages/transition-backend/src/services/gtfsExport/__tests__/ScheduleExporter.test.ts @@ -69,19 +69,22 @@ const pathDistances = path.getCoordinatesDistanceTraveledMeters().map(dist => Ma const lineId = uuidV4(); const serviceId = uuidV4(); const serviceId2 = uuidV4(); +const scheduleIntegerId = 4; // Schedule with many periods/trips, but one path const scheduleAttributes1: ScheduleAttributes = { allow_seconds_based_schedules: false, id: uuidV4(), + integer_id: scheduleIntegerId, line_id: lineId, service_id: serviceId, is_frozen: false, data: {}, periods: [{ // Period with start and end hours and multiple trips - schedule_id: uuidV4(), + schedule_id: scheduleIntegerId, id: uuidV4(), + integer_id: 1, data: {}, end_at_hour: 12, inbound_path_id: undefined, @@ -102,8 +105,7 @@ const scheduleAttributes1: ScheduleAttributes = { path_id: pathAttributes.id, seated_capacity: 20, total_capacity: 50, - schedule_id: uuidV4(), - schedule_period_id: uuidV4(), + schedule_period_id: 1, data: {} }, { arrival_time_seconds: 32416, @@ -116,8 +118,7 @@ const scheduleAttributes1: ScheduleAttributes = { path_id: pathAttributes.id, seated_capacity: 20, total_capacity: 50, - schedule_id: uuidV4(), - schedule_period_id: uuidV4(), + schedule_period_id: 1, data: {} }, { arrival_time_seconds: 34216, @@ -130,13 +131,13 @@ const scheduleAttributes1: ScheduleAttributes = { path_id: pathAttributes.id, seated_capacity: 20, total_capacity: 50, - schedule_id: uuidV4(), - schedule_period_id: uuidV4(), + schedule_period_id: 1, data: {} }] }, { - schedule_id: uuidV4(), + schedule_id: scheduleIntegerId, id: uuidV4(), + integer_id: 4, data: {}, custom_start_at_str: "13:15", custom_end_at_str: "17:24", @@ -156,13 +157,12 @@ const scheduleAttributes1: ScheduleAttributes = { path_id: pathAttributes.id, seated_capacity: 20, total_capacity: 50, - schedule_id: uuidV4(), - schedule_period_id: uuidV4(), + schedule_period_id: 4, data: {} }] }, { // Period with custom start and end, without trips - schedule_id: uuidV4(), + schedule_id: scheduleIntegerId, id: uuidV4(), data: {}, custom_start_at_str: "18:00", @@ -178,16 +178,19 @@ const scheduleAttributes1: ScheduleAttributes = { }; // Simple second schedule, one trip in each direction, the actual coordinates and routability are not important +const scheduleIntegerId2 = 5; const scheduleAttributes2: ScheduleAttributes = { allow_seconds_based_schedules: false, id: uuidV4(), + integer_id: scheduleIntegerId2, line_id: lineId, service_id: serviceId2, is_frozen: false, data: {}, periods: [{ - schedule_id: uuidV4(), + schedule_id: scheduleIntegerId2, id: uuidV4(), + integer_id: 2, data: {}, end_at_hour: 12, inbound_path_id: pathAttributes2.id, @@ -207,8 +210,7 @@ const scheduleAttributes2: ScheduleAttributes = { path_id: pathAttributes.id, seated_capacity: 20, total_capacity: 50, - schedule_id: uuidV4(), - schedule_period_id: uuidV4(), + schedule_period_id: 2, data: {} }, { @@ -223,8 +225,7 @@ const scheduleAttributes2: ScheduleAttributes = { path_id: pathAttributes2.id, seated_capacity: 20, total_capacity: 50, - schedule_id: uuidV4(), - schedule_period_id: uuidV4(), + schedule_period_id: 2, data: {} }] }], diff --git a/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts b/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts index 63e78d197..1ffe43b01 100644 --- a/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts +++ b/packages/transition-backend/src/services/gtfsImport/ScheduleImporter.ts @@ -229,7 +229,7 @@ const createSchedule = ( const period = periods[j]; const periodSchedule = { id: uuidV4(), - schedule_id: schedule.getId(), + schedule_id: schedule.attributes.integer_id, start_at_hour: period.startAtHour, end_at_hour: period.endAtHour, period_shortname: period.shortname, @@ -332,8 +332,7 @@ const generateExactTrips = (schedule: Schedule, periods: TripAndStopTimes[][], i const newTrip = { id: uuidV4(), - schedule_id: schedule.getId(), - schedule_period_id: schedule.getAttributes().periods[periodIndex].id, + schedule_period_id: schedule.getAttributes().periods[periodIndex].integer_id, path_id: pathId, departure_time_seconds: tripDepartureTimeSeconds, arrival_time_seconds: tripArrivalTimeSeconds, @@ -478,9 +477,14 @@ const generateSchedulesForLine = async ( } line.addSchedule(schedule); - if (existingSchedules[schedule.attributes.service_id]) { + if ( + existingSchedules[schedule.attributes.service_id] && + typeof existingSchedules[schedule.attributes.service_id].attributes.integer_id === 'number' + ) { // Delete previous schedules for this service - await scheduleDbQueries.delete(existingSchedules[schedule.attributes.service_id].getId()); + await scheduleDbQueries.delete( + existingSchedules[schedule.attributes.service_id].attributes.integer_id as number + ); } } diff --git a/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts b/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts index c2177773c..cfc59545d 100644 --- a/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts +++ b/packages/transition-backend/src/services/gtfsImport/__tests__/ScheduleImporter.test.ts @@ -351,15 +351,14 @@ describe('Generate schedules for lines', () => { })); expect(scheduleAttributes.periods.length).toEqual(2); expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, })); expect(scheduleAttributes.periods[0].trips.length).toEqual(2); expect(scheduleAttributes.periods[0].trips[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[0].id, + schedule_period_id: scheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [baseTripAndStopTimes.stopTimes[0].departureTimeSeconds, baseTripAndStopTimes.stopTimes[1].departureTimeSeconds, baseTripAndStopTimes.stopTimes[2].departureTimeSeconds, baseTripAndStopTimes.stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [baseTripAndStopTimes.stopTimes[0].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[1].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[2].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[3].arrivalTimeSeconds], @@ -369,8 +368,7 @@ describe('Generate schedules for lines', () => { nodes_can_unboard: [false, true, true, true], })); expect(scheduleAttributes.periods[0].trips[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[0].id, + schedule_period_id: scheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [tripsByRouteId[routeId][1].stopTimes[0].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[1].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[2].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [tripsByRouteId[routeId][1].stopTimes[0].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[1].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[2].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[3].arrivalTimeSeconds], @@ -380,7 +378,6 @@ describe('Generate schedules for lines', () => { nodes_can_unboard: [false, false, true, true], })); expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -432,15 +429,14 @@ describe('Generate schedules for lines', () => { expect(scheduleAttributes.periods.length).toEqual(2); // Check first period expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname })); expect(scheduleAttributes.periods[0].trips.length).toEqual(1); expect(scheduleAttributes.periods[0].trips[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[0].id, + schedule_period_id: scheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [baseTripAndStopTimes.stopTimes[0].departureTimeSeconds, baseTripAndStopTimes.stopTimes[1].departureTimeSeconds, baseTripAndStopTimes.stopTimes[2].departureTimeSeconds, baseTripAndStopTimes.stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [baseTripAndStopTimes.stopTimes[0].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[1].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[2].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[3].arrivalTimeSeconds], @@ -452,15 +448,14 @@ describe('Generate schedules for lines', () => { // Check second period expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, })); expect(scheduleAttributes.periods[1].trips.length).toEqual(2); expect(scheduleAttributes.periods[1].trips[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[1].id, + schedule_period_id: scheduleAttributes.periods[1].integer_id, path_id: pathId, node_departure_times_seconds: [tripsByRouteId[routeId][1].stopTimes[0].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[1].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[2].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [tripsByRouteId[routeId][1].stopTimes[0].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[1].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[2].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[3].arrivalTimeSeconds], @@ -470,8 +465,7 @@ describe('Generate schedules for lines', () => { nodes_can_unboard: [false, true, true, true], })); expect(scheduleAttributes.periods[1].trips[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[1].id, + schedule_period_id: scheduleAttributes.periods[1].integer_id, path_id: pathId, node_departure_times_seconds: [tripsByRouteId[routeId][2].stopTimes[0].departureTimeSeconds, tripsByRouteId[routeId][2].stopTimes[1].departureTimeSeconds, tripsByRouteId[routeId][2].stopTimes[2].departureTimeSeconds, tripsByRouteId[routeId][2].stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [tripsByRouteId[routeId][2].stopTimes[0].arrivalTimeSeconds, tripsByRouteId[routeId][2].stopTimes[1].arrivalTimeSeconds, tripsByRouteId[routeId][2].stopTimes[2].arrivalTimeSeconds, tripsByRouteId[routeId][2].stopTimes[3].arrivalTimeSeconds], @@ -524,14 +518,13 @@ describe('Generate schedules for lines', () => { periods_group_shortname: importData.periodsGroupShortname, periods: [ expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, trips: [ expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[0].id, + schedule_period_id: scheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [baseTripAndStopTimes.stopTimes[0].departureTimeSeconds, baseTripAndStopTimes.stopTimes[1].departureTimeSeconds, baseTripAndStopTimes.stopTimes[2].departureTimeSeconds, baseTripAndStopTimes.stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [baseTripAndStopTimes.stopTimes[0].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[1].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[2].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[3].arrivalTimeSeconds], @@ -543,7 +536,7 @@ describe('Generate schedules for lines', () => { ], }), expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -561,14 +554,13 @@ describe('Generate schedules for lines', () => { periods_group_shortname: importData.periodsGroupShortname, periods: [ expect.objectContaining({ - schedule_id: secondScheduleAttributes.id, + schedule_id: secondScheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, trips: [ expect.objectContaining({ - schedule_id: secondScheduleAttributes.id, - schedule_period_id: secondScheduleAttributes.periods[0].id, + schedule_period_id: secondScheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [tripsByRouteId[routeId][1].stopTimes[0].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[1].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[2].departureTimeSeconds, tripsByRouteId[routeId][1].stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [tripsByRouteId[routeId][1].stopTimes[0].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[1].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[2].arrivalTimeSeconds, tripsByRouteId[routeId][1].stopTimes[3].arrivalTimeSeconds], @@ -580,7 +572,7 @@ describe('Generate schedules for lines', () => { ], }), expect.objectContaining({ - schedule_id: secondScheduleAttributes.id, + schedule_id: secondScheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -649,14 +641,13 @@ describe('Generate schedules for lines', () => { periods_group_shortname: importData.periodsGroupShortname, periods: [ expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, trips: [ expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[0].id, + schedule_period_id: scheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [baseTripAndStopTimes.stopTimes[0].departureTimeSeconds, baseTripAndStopTimes.stopTimes[1].departureTimeSeconds, baseTripAndStopTimes.stopTimes[2].departureTimeSeconds, baseTripAndStopTimes.stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [baseTripAndStopTimes.stopTimes[0].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[1].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[2].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[3].arrivalTimeSeconds], @@ -668,7 +659,7 @@ describe('Generate schedules for lines', () => { ], }), expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -686,14 +677,13 @@ describe('Generate schedules for lines', () => { periods_group_shortname: importData.periodsGroupShortname, periods: [ expect.objectContaining({ - schedule_id: secondScheduleAttributes.id, + schedule_id: secondScheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, trips: [ expect.objectContaining({ - schedule_id: secondScheduleAttributes.id, - schedule_period_id: secondScheduleAttributes.periods[0].id, + schedule_period_id: secondScheduleAttributes.periods[0].integer_id, path_id: pathId, node_departure_times_seconds: [tripsByRouteId[routeId][0].stopTimes[0].departureTimeSeconds, tripsByRouteId[routeId][0].stopTimes[1].departureTimeSeconds, tripsByRouteId[routeId][0].stopTimes[2].departureTimeSeconds, tripsByRouteId[routeId][0].stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [tripsByRouteId[routeId][0].stopTimes[0].arrivalTimeSeconds, tripsByRouteId[routeId][0].stopTimes[1].arrivalTimeSeconds, tripsByRouteId[routeId][0].stopTimes[2].arrivalTimeSeconds, tripsByRouteId[routeId][0].stopTimes[3].arrivalTimeSeconds], @@ -705,7 +695,7 @@ describe('Generate schedules for lines', () => { ], }), expect.objectContaining({ - schedule_id: secondScheduleAttributes.id, + schedule_id: secondScheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -777,14 +767,13 @@ describe('Generate schedules for lines', () => { periods_group_shortname: importData.periodsGroupShortname, periods: [ expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, trips: [ expect.objectContaining({ - schedule_id: scheduleAttributes.id, - schedule_period_id: scheduleAttributes.periods[0].id, + schedule_period_id: scheduleAttributes.periods[0].integer_id, path_id: pathId1, node_departure_times_seconds: [baseTripAndStopTimes.stopTimes[0].departureTimeSeconds, baseTripAndStopTimes.stopTimes[1].departureTimeSeconds, baseTripAndStopTimes.stopTimes[2].departureTimeSeconds, baseTripAndStopTimes.stopTimes[3].departureTimeSeconds], node_arrival_times_seconds: [baseTripAndStopTimes.stopTimes[0].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[1].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[2].arrivalTimeSeconds, baseTripAndStopTimes.stopTimes[3].arrivalTimeSeconds], @@ -796,7 +785,7 @@ describe('Generate schedules for lines', () => { ], }), expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -814,14 +803,13 @@ describe('Generate schedules for lines', () => { periods_group_shortname: importData.periodsGroupShortname, periods: [ expect.objectContaining({ - schedule_id: scheduleAttributesLine2.id, + schedule_id: scheduleAttributesLine2.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, trips: [ expect.objectContaining({ - schedule_id: scheduleAttributesLine2.id, - schedule_period_id: scheduleAttributesLine2.periods[0].id, + schedule_period_id: scheduleAttributesLine2.periods[0].integer_id, path_id: pathId2, node_departure_times_seconds: [tripsByRouteId[expressRouteId][0].stopTimes[0].departureTimeSeconds, tripsByRouteId[expressRouteId][0].stopTimes[1].departureTimeSeconds], node_arrival_times_seconds: [tripsByRouteId[expressRouteId][0].stopTimes[0].arrivalTimeSeconds, tripsByRouteId[expressRouteId][0].stopTimes[1].arrivalTimeSeconds], @@ -833,7 +821,7 @@ describe('Generate schedules for lines', () => { ], }), expect.objectContaining({ - schedule_id: scheduleAttributesLine2.id, + schedule_id: scheduleAttributesLine2.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -869,6 +857,7 @@ describe('Generate schedules for lines', () => { const previousScheduleId = uuidV4(); line.attributes.scheduleByServiceId[serviceId] = { id: previousScheduleId, + integer_id: 4, line_id: line.getId(), service_id: serviceId, periods: [], @@ -992,7 +981,7 @@ describe('Generate frequency based schedules for line', () => { })); expect(scheduleAttributes.periods.length).toEqual(2); expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, @@ -1003,7 +992,7 @@ describe('Generate frequency based schedules for line', () => { inbound_path_id: undefined })); expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -1062,7 +1051,7 @@ describe('Generate frequency based schedules for line', () => { })); expect(scheduleAttributes.periods.length).toEqual(2); expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, @@ -1074,7 +1063,7 @@ describe('Generate frequency based schedules for line', () => { inbound_path_id: undefined })); expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -1149,7 +1138,7 @@ describe('Generate frequency based schedules for line', () => { })); expect(scheduleAttributes.periods.length).toEqual(2); expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, @@ -1161,7 +1150,7 @@ describe('Generate frequency based schedules for line', () => { inbound_path_id: undefined })); expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -1231,7 +1220,7 @@ describe('Generate frequency based schedules for line', () => { })); expect(scheduleAttributes.periods.length).toEqual(2); expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, @@ -1240,7 +1229,7 @@ describe('Generate frequency based schedules for line', () => { inbound_path_id: inboundPathId })); expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, @@ -1322,7 +1311,7 @@ describe('Generate frequency based schedules for line', () => { })); expect(scheduleAttributes.periods.length).toEqual(2); expect(scheduleAttributes.periods[0]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[0].startAtHour, end_at_hour: importData.periodsGroup.periods[0].endAtHour, period_shortname: importData.periodsGroup.periods[0].shortname, @@ -1331,7 +1320,7 @@ describe('Generate frequency based schedules for line', () => { inbound_path_id: inboundPathId })); expect(scheduleAttributes.periods[1]).toEqual(expect.objectContaining({ - schedule_id: scheduleAttributes.id, + schedule_id: scheduleAttributes.integer_id, start_at_hour: importData.periodsGroup.periods[1].startAtHour, end_at_hour: importData.periodsGroup.periods[1].endAtHour, period_shortname: importData.periodsGroup.periods[1].shortname, diff --git a/packages/transition-common/src/services/schedules/Schedule.ts b/packages/transition-common/src/services/schedules/Schedule.ts index 883867e93..1e64c135b 100644 --- a/packages/transition-common/src/services/schedules/Schedule.ts +++ b/packages/transition-common/src/services/schedules/Schedule.ts @@ -19,8 +19,7 @@ import SaveUtils from 'chaire-lib-common/lib/services/objects/SaveUtils'; import CollectionManager from 'chaire-lib-common/lib/utils/objects/CollectionManager'; export interface SchedulePeriodTrip extends GenericAttributes { - schedule_id: string; - schedule_period_id: string; + schedule_period_id?: number; path_id: string; unit_id?: string; block_id?: string; @@ -35,7 +34,7 @@ export interface SchedulePeriodTrip extends GenericAttributes { } export interface SchedulePeriod extends GenericAttributes { - schedule_id: string; + schedule_id?: number; outbound_path_id?: string; inbound_path_id?: string; period_shortname?: string; @@ -109,11 +108,13 @@ class Schedule extends ObjectWithHistory implements Saveable getClonedAttributes(deleteSpecifics = true): Partial { const clonedAttributes = super.getClonedAttributes(deleteSpecifics); if (deleteSpecifics) { + delete clonedAttributes.integer_id; const periods = clonedAttributes.periods; if (periods) { for (let i = 0; i < periods.length; i++) { const period = periods[i] as Partial; delete period.id; + delete period.integer_id; delete period.schedule_id; delete period.created_at; delete period.updated_at; @@ -122,7 +123,7 @@ class Schedule extends ObjectWithHistory implements Saveable for (let j = 0; j < trips.length; j++) { const trip = trips[j] as Partial; delete trip.id; - delete trip.schedule_id; + delete trip.integer_id; delete trip.schedule_period_id; delete trip.created_at; delete trip.updated_at; diff --git a/packages/transition-common/src/services/schedules/ScheduleDuplicator.ts b/packages/transition-common/src/services/schedules/ScheduleDuplicator.ts index 37e4e6e74..db63b6c53 100644 --- a/packages/transition-common/src/services/schedules/ScheduleDuplicator.ts +++ b/packages/transition-common/src/services/schedules/ScheduleDuplicator.ts @@ -16,6 +16,8 @@ export interface DuplicateScheduleOptions { /** * duplicate a schedule object, with all trips and periods + * + * FIXME Will be moved to backend for easier copy of schedules */ export const duplicateSchedule = async ( baseSchedule: Schedule, @@ -23,6 +25,7 @@ export const duplicateSchedule = async ( ): Promise => { const newScheduleAttribs = baseSchedule.getClonedAttributes(true); newScheduleAttribs.id = uuidV4(); + delete newScheduleAttribs.integer_id; if (serviceId) newScheduleAttribs.service_id = serviceId; if (lineId) newScheduleAttribs.line_id = lineId; @@ -31,7 +34,7 @@ export const duplicateSchedule = async ( for (let periodI = 0, countPeriods = newScheduleAttribs.periods.length; periodI < countPeriods; periodI++) { const period = newScheduleAttribs.periods[periodI]; period.id = uuidV4(); - period.schedule_id = newScheduleAttribs.id; + delete period.schedule_id; if (period.inbound_path_id && pathIdsMapping[period.inbound_path_id]) { period.inbound_path_id = pathIdsMapping[period.inbound_path_id]; @@ -43,8 +46,7 @@ export const duplicateSchedule = async ( for (let tripI = 0, countTrips = period.trips.length; tripI < countTrips; tripI++) { const trip = period.trips[tripI]; trip.id = uuidV4(); - trip.schedule_period_id = period.id; - trip.schedule_id = newScheduleAttribs.id; + delete trip.schedule_period_id; if (trip.path_id && pathIdsMapping[trip.path_id]) { trip.path_id = pathIdsMapping[trip.path_id]; } diff --git a/packages/transition-common/src/services/schedules/__tests__/Schedule.test.ts b/packages/transition-common/src/services/schedules/__tests__/Schedule.test.ts index 5b1c2f65c..b67dbddc2 100644 --- a/packages/transition-common/src/services/schedules/__tests__/Schedule.test.ts +++ b/packages/transition-common/src/services/schedules/__tests__/Schedule.test.ts @@ -243,7 +243,7 @@ describe('generateForPeriod', () => { const smallPeriodsForUpdate: SchedulePeriod[] = [{ // Period with start and end hours and multiple trips id: uuidV4(), - schedule_id: scheduleAttributesForUpdate.id, + schedule_id: scheduleAttributesForUpdate.integer_id, inbound_path_id: undefined, outbound_path_id: path.getId(), period_shortname: "period1", @@ -253,7 +253,7 @@ describe('generateForPeriod', () => { trips: [] }, { id: uuidV4(), - schedule_id: scheduleAttributesForUpdate.id, + schedule_id: scheduleAttributesForUpdate.integer_id, inbound_path_id: undefined, outbound_path_id: path.getId(), period_shortname: "period2", diff --git a/packages/transition-common/src/services/schedules/__tests__/ScheduleData.test.ts b/packages/transition-common/src/services/schedules/__tests__/ScheduleData.test.ts index a22638757..8e769b74b 100644 --- a/packages/transition-common/src/services/schedules/__tests__/ScheduleData.test.ts +++ b/packages/transition-common/src/services/schedules/__tests__/ScheduleData.test.ts @@ -12,7 +12,8 @@ const defaultLineId = uuidV4(); const defaultServiceId = uuidV4(); const defaultPathId = uuidV4(); const defaultScheduleId = uuidV4(); -const periodIds = [uuidV4(), uuidV4(), uuidV4()]; +const periodIds = [1, 2, 3]; +const scheduleIntegerId = 1; // FIXME: departure/arrival times need to be set to number[] even if they have null. Make sure ScheduleAttributes have the proper types or review how times are used. export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ @@ -23,6 +24,7 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ }) => { return { id: scheduleId, + integer_id: scheduleIntegerId, allow_seconds_based_schedules: false, line_id: lineId, service_id: serviceId, @@ -31,8 +33,9 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ data: {}, periods: [{ // Period with start and end hours and multiple trips - id: periodIds[0], - schedule_id: scheduleId, + id: uuidV4(), + integer_id: periodIds[0], + schedule_id: scheduleIntegerId, inbound_path_id: undefined, outbound_path_id: pathId, interval_seconds: 1800, @@ -42,7 +45,7 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ data: {}, trips: [{ id: uuidV4(), - schedule_id: scheduleId, + integer_id: 1, schedule_period_id: periodIds[0], data: {}, path_id: pathId, @@ -56,7 +59,7 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ total_capacity: 50 }, { id: uuidV4(), - schedule_id: scheduleId, + integer_id: 2, schedule_period_id: periodIds[0], data: {}, path_id: pathId, @@ -70,7 +73,7 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ total_capacity: 50 }, { id: uuidV4(), - schedule_id: scheduleId, + integer_id: 3, schedule_period_id: periodIds[0], data: {}, path_id: pathId, @@ -84,8 +87,9 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ total_capacity: 50 }] }, { - id: periodIds[1], - schedule_id: scheduleId, + id: uuidV4(), + integer_id: periodIds[1], + schedule_id: scheduleIntegerId, // Period with custom start and end, with a single trip custom_start_at_str: "13:15", custom_end_at_str: "17:24", @@ -98,7 +102,7 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ data: {}, trips: [{ id: uuidV4(), - schedule_id: scheduleId, + integer_id: 4, schedule_period_id: periodIds[1], data: {}, path_id: pathId, @@ -113,8 +117,9 @@ export const getScheduleAttributes: (params: any) => ScheduleAttributes = ({ }] }, { // Period with custom start and end, without trips - id: periodIds[2], - schedule_id: scheduleId, + id: uuidV4(), + integer_id: periodIds[2], + schedule_id: scheduleIntegerId, data: {}, custom_start_at_str: '18:00', custom_end_at_str: '23:00', diff --git a/packages/transition-common/src/services/schedules/__tests__/ScheduleDuplicator.test.ts b/packages/transition-common/src/services/schedules/__tests__/ScheduleDuplicator.test.ts index 7461766c4..0fe7529c7 100644 --- a/packages/transition-common/src/services/schedules/__tests__/ScheduleDuplicator.test.ts +++ b/packages/transition-common/src/services/schedules/__tests__/ScheduleDuplicator.test.ts @@ -24,7 +24,7 @@ test('Duplicate schedule, same line path and services', async () => { // Validate the schedule's data const expectedSched = getScheduleAttributes({ pathId, scheduleId: newSchedule.getId() }); const actualSched = newSchedule.getAttributes(); - const expectedBaseSchedules = _omit(expectedSched, 'periods'); + const expectedBaseSchedules = _omit(expectedSched, 'periods', 'integer_id'); const actualSchedule = _omit(actualSched, 'periods'); const expectedPeriods = expectedSched.periods; const actualPeriods = actualSched.periods; @@ -32,18 +32,17 @@ test('Duplicate schedule, same line path and services', async () => { // Validate the period's data and id propagation for (let i = 0; i < expectedPeriods.length; i++) { - const expectedPeriod = _omit(expectedPeriods[i], ['id', 'trips']); + const expectedPeriod = _omit(expectedPeriods[i], ['id', 'trips', 'integer_id', 'schedule_id']); const actualPeriod = _omit(actualPeriods[i], ['id', 'trips']); const expectedTrips = expectedPeriods[i].trips; const actualTrips = actualPeriods[i].trips; - const periodId = actualPeriods[i].id; + const periodId = actualPeriods[i].integer_id; expect(actualPeriod).toEqual(expectedPeriod); // Validate the trip's data and id propagation for (let j = 0; j < expectedTrips.length; j++) { - const expectedTrip = _omit(expectedTrips[j], 'id'); + const expectedTrip = _omit(expectedTrips[j], 'id', 'integer_id', 'schedule_period_id'); const actualTrip = _omit(actualTrips[j], 'id'); - expectedTrip.schedule_period_id = periodId; expect(actualTrip).toEqual(expectedTrip); } } @@ -65,7 +64,7 @@ test('Duplicate schedule, different line, path and services', async () => { // Validate the schedule's data const expectedSched = getScheduleAttributes({ pathId: newPathId, lineId: newLineId, serviceId: newServiceId, scheduleId: newSchedule.getId() }); const actualSched = newSchedule.getAttributes(); - const expectedBaseSchedules = _omit(expectedSched, 'periods'); + const expectedBaseSchedules = _omit(expectedSched, 'periods', 'integer_id'); const actualSchedule = _omit(actualSched, 'periods'); const expectedPeriods = expectedSched.periods; const actualPeriods = actualSched.periods; @@ -73,19 +72,17 @@ test('Duplicate schedule, different line, path and services', async () => { // Validate the period's data and id propagation for (let i = 0; i < expectedPeriods.length; i++) { - const expectedPeriod = _omit(expectedPeriods[i], ['id', 'trips']); + const expectedPeriod = _omit(expectedPeriods[i], ['id', 'trips', 'integer_id', 'schedule_id']); const actualPeriod = _omit(actualPeriods[i], ['id', 'trips']); expect(actualPeriod.outbound_path_id).not.toEqual(schedule.getAttributes().periods[i].outbound_path_id); const expectedTrips = expectedPeriods[i].trips; const actualTrips = actualPeriods[i].trips; - const periodId = actualPeriods[i].id; expect(actualPeriod).toEqual(expectedPeriod); // Validate the trip's data and id propagation for (let j = 0; j < expectedTrips.length; j++) { - const expectedTrip = _omit(expectedTrips[j], 'id'); + const expectedTrip = _omit(expectedTrips[j], 'id', 'integer_id', 'schedule_period_id'); const actualTrip = _omit(actualTrips[j], 'id'); - expectedTrip.schedule_period_id = periodId; expect(actualTrip).toEqual(expectedTrip); } }