From 095d0131000373cfc065d33cfcefecd5260cb820 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Fri, 21 Jan 2022 08:36:06 -0500 Subject: [PATCH 1/4] fix(core-utils): more resiliently expand flex mode --- packages/core-utils/src/query.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/core-utils/src/query.js b/packages/core-utils/src/query.js index 1f1d8432d..cb5801ae9 100644 --- a/packages/core-utils/src/query.js +++ b/packages/core-utils/src/query.js @@ -466,8 +466,19 @@ export function getRoutingParams(config, currentQuery, ignoreRealtimeUpdates) { // Replace FLEX placeholder with OTP flex modes if (params.mode) { // Ensure query is in reduced format to avoid replacing twice - const mode = reduceOtpFlexModes(params.mode.split(",")).join(","); - params.mode = mode.replace("FLEX", "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT"); + const mode = reduceOtpFlexModes(params.mode.split(",")); + params.mode = mode + .map(m => { + // If both the expanded and shrunk modes are included, remove the exapnded one + if (m === "FLEX_EGRESS" || m === "FLEX_ACCESS" || m === "FLEX_DIRECT") { + if (mode.includes("FLEX")) return ""; + } + if (m === "FLEX") { + return "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT"; + } + return m; + }) + .join(","); } return params; From 70d83757ca12f50aea4417d847732854c6c660b8 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Tue, 25 Jan 2022 09:19:24 -0500 Subject: [PATCH 2/4] test: add unit tests for flex reducers --- .../__snapshots__/query-params.js.snap | 47 ++++++++ .../src/__tests__/__snapshots__/query.js.snap | 100 ++++++++++++++++++ .../core-utils/src/__tests__/query-params.js | 49 +++++++++ packages/core-utils/src/__tests__/query.js | 52 +++++++++ packages/core-utils/src/query.js | 37 ++++--- 5 files changed, 272 insertions(+), 13 deletions(-) diff --git a/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap b/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap index 4863691a9..05de93dda 100644 --- a/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap +++ b/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap @@ -1,5 +1,52 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`flex-reducer should modify a query that includes all flex modes 1`] = ` +Array [ + "WALK", + "TRANSIT", + "BIKE", + "FLEX", +] +`; + +exports[`flex-reducer should modify a query that includes all flex modes 2`] = ` +Array [ + "FLEX", + "BIKE", + "WALK", + "TRANSIT", +] +`; + +exports[`flex-reducer should modify a query that includes duplicate flex modes 1`] = ` +Array [ + "FLEX", +] +`; + +exports[`flex-reducer should modify a query that includes only flex modes 1`] = ` +Array [ + "FLEX", +] +`; + +exports[`flex-reducer should modify a query that includes some flex modes 1`] = ` +Array [ + "WALK", + "TRANSIT", + "BIKE", + "FLEX", +] +`; + +exports[`flex-reducer should not touch a query that doesn't include flex modes 1`] = ` +Array [ + "WALK", + "TRANSIT", + "BIKE", +] +`; + exports[`query-params getCustomQueryParams should return queryParams with customizations 1`] = ` Array [ Object { diff --git a/packages/core-utils/src/__tests__/__snapshots__/query.js.snap b/packages/core-utils/src/__tests__/__snapshots__/query.js.snap index 8d52797df..76b93a2e2 100644 --- a/packages/core-utils/src/__tests__/__snapshots__/query.js.snap +++ b/packages/core-utils/src/__tests__/__snapshots__/query.js.snap @@ -60,6 +60,58 @@ Object { } `; +exports[`query getRoutingParams should create repaired routing params for broken flex query 1`] = ` +Object { + "arriveBy": false, + "date": "2020-08-24", + "fromPlace": "Steamer Lane, Santa Cruz, CA, 95060, USA::36.95471026341096,-122.0248185852425", + "ignoreRealtimeUpdates": false, + "maxWalkDistance": 1207, + "mode": "WALK,FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT,TRANSIT", + "numItineraries": 3, + "optimize": "QUICK", + "otherThanPreferredRoutesPenalty": 900, + "showIntermediateStops": true, + "time": "21:53", + "toPlace": "Municipal Wharf St, Santa Cruz, CA, 95060, USA::36.961843106786766,-122.02402657342725", + "walkSpeed": 1.34, +} +`; + +exports[`query getRoutingParams should create repaired routing params for very broken flex query 1`] = ` +Object { + "arriveBy": false, + "date": "2020-08-24", + "fromPlace": "Steamer Lane, Santa Cruz, CA, 95060, USA::36.95471026341096,-122.0248185852425", + "ignoreRealtimeUpdates": false, + "mode": "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT,WALK,BIKE", + "numItineraries": 3, + "otherThanPreferredRoutesPenalty": 900, + "showIntermediateStops": true, + "time": "21:53", + "toPlace": "Municipal Wharf St, Santa Cruz, CA, 95060, USA::36.961843106786766,-122.02402657342725", + "walkSpeed": 1.34, +} +`; + +exports[`query getRoutingParams should create repaired routing params for very broken flex query 2`] = ` +Object { + "arriveBy": false, + "date": "2020-08-24", + "fromPlace": "Steamer Lane, Santa Cruz, CA, 95060, USA::36.95471026341096,-122.0248185852425", + "ignoreRealtimeUpdates": false, + "maxWalkDistance": 1207, + "mode": "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT,WALK,BIKE,WALK,TRANSIT,BUS", + "numItineraries": 3, + "optimize": "QUICK", + "otherThanPreferredRoutesPenalty": 900, + "showIntermediateStops": true, + "time": "21:53", + "toPlace": "Municipal Wharf St, Santa Cruz, CA, 95060, USA::36.961843106786766,-122.02402657342725", + "walkSpeed": 1.34, +} +`; + exports[`query getRoutingParams should create routing params for car rental query 1`] = ` Object { "arriveBy": false, @@ -80,6 +132,21 @@ Object { } `; +exports[`query getRoutingParams should create routing params for flex-only query 1`] = ` +Object { + "arriveBy": false, + "date": "2020-08-24", + "fromPlace": "Steamer Lane, Santa Cruz, CA, 95060, USA::36.95471026341096,-122.0248185852425", + "ignoreRealtimeUpdates": false, + "mode": "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT", + "numItineraries": 3, + "otherThanPreferredRoutesPenalty": 900, + "showIntermediateStops": true, + "time": "21:53", + "toPlace": "Municipal Wharf St, Santa Cruz, CA, 95060, USA::36.961843106786766,-122.02402657342725", +} +`; + exports[`query getRoutingParams should create routing params for intermediate places query 1`] = ` Object { "arriveBy": false, @@ -124,6 +191,39 @@ Object { } `; +exports[`query getRoutingParams should create routing params for standard flex query 1`] = ` +Object { + "arriveBy": false, + "date": "2020-08-24", + "fromPlace": "Steamer Lane, Santa Cruz, CA, 95060, USA::36.95471026341096,-122.0248185852425", + "ignoreRealtimeUpdates": false, + "maxWalkDistance": 1207, + "mode": "WALK,TRANSIT,FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT", + "numItineraries": 3, + "optimize": "QUICK", + "otherThanPreferredRoutesPenalty": 900, + "showIntermediateStops": true, + "time": "21:53", + "toPlace": "Municipal Wharf St, Santa Cruz, CA, 95060, USA::36.961843106786766,-122.02402657342725", + "walkSpeed": 1.34, +} +`; + +exports[`query getRoutingParams should create routing params for standard flex query 2`] = ` +Object { + "arriveBy": false, + "date": "2020-08-24", + "fromPlace": "Steamer Lane, Santa Cruz, CA, 95060, USA::36.95471026341096,-122.0248185852425", + "ignoreRealtimeUpdates": false, + "mode": "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT", + "numItineraries": 3, + "otherThanPreferredRoutesPenalty": 900, + "showIntermediateStops": true, + "time": "21:53", + "toPlace": "Municipal Wharf St, Santa Cruz, CA, 95060, USA::36.961843106786766,-122.02402657342725", +} +`; + exports[`query getRoutingParams should create routing params for transit query 1`] = ` Object { "arriveBy": false, diff --git a/packages/core-utils/src/__tests__/query-params.js b/packages/core-utils/src/__tests__/query-params.js index 1ad5d7b93..afecc3439 100644 --- a/packages/core-utils/src/__tests__/query-params.js +++ b/packages/core-utils/src/__tests__/query-params.js @@ -1,3 +1,4 @@ +import { reduceOtpFlexModes } from "../query"; import queryParams, { getCustomQueryParams } from "../query-params"; const customWalkDistanceOptions = [ @@ -38,3 +39,51 @@ describe("query-params", () => { }); }); }); + +describe("flex-reducer", () => { + it("should not touch a query that doesn't include flex modes", () => { + expect(reduceOtpFlexModes(["WALK", "TRANSIT", "BIKE"])).toMatchSnapshot(); + }); + it("should modify a query that includes some flex modes", () => { + expect( + reduceOtpFlexModes(["WALK", "TRANSIT", "BIKE", "FLEX_DIRECT"]) + ).toMatchSnapshot(); + }); + it("should modify a query that includes all flex modes", () => { + expect( + reduceOtpFlexModes([ + "WALK", + "TRANSIT", + "BIKE", + "FLEX_DIRECT", + "FLEX_ACCESS", + "FLEX_EGRESS" + ]) + ).toMatchSnapshot(); + expect( + reduceOtpFlexModes([ + "FLEX_DIRECT", + "BIKE", + "FLEX_ACCESS", + "WALK", + "FLEX_EGRESS", + "TRANSIT" + ]) + ).toMatchSnapshot(); + }); + it("should modify a query that includes only flex modes", () => { + expect( + reduceOtpFlexModes(["FLEX_DIRECT", "FLEX_ACCESS", "FLEX_EGRESS"]) + ).toMatchSnapshot(); + }); + it("should modify a query that includes duplicate flex modes", () => { + expect( + reduceOtpFlexModes([ + "FLEX_DIRECT", + "FLEX_DIRECT", + "FLEX_ACCESS", + "FLEX_EGRESS" + ]) + ).toMatchSnapshot(); + }); +}); diff --git a/packages/core-utils/src/__tests__/query.js b/packages/core-utils/src/__tests__/query.js index 0dbaf6c05..ebc217257 100644 --- a/packages/core-utils/src/__tests__/query.js +++ b/packages/core-utils/src/__tests__/query.js @@ -102,6 +102,58 @@ describe("query", () => { }) ).toMatchSnapshot(); }); + + it("should create routing params for standard flex query", () => { + expect( + getRoutingParams(fakeConfig, { + ...makeBaseTestQuery(), + mode: "WALK,TRANSIT,FLEX" + }) + ).toMatchSnapshot(); + }); + + it("should create repaired routing params for broken flex query", () => { + expect( + getRoutingParams(fakeConfig, { + ...makeBaseTestQuery(), + mode: "WALK,FLEX_DIRECT,TRANSIT,FLEX" + }) + ).toMatchSnapshot(); + }); + + it("should create routing params for flex-only query", () => { + expect( + getRoutingParams(fakeConfig, { + ...makeBaseTestQuery(), + mode: "FLEX" + }) + ).toMatchSnapshot(); + }); + + it("should create routing params for standard flex query", () => { + expect( + getRoutingParams(fakeConfig, { + ...makeBaseTestQuery(), + mode: "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT" + }) + ).toMatchSnapshot(); + }); + + it("should create repaired routing params for very broken flex query", () => { + expect( + getRoutingParams(fakeConfig, { + ...makeBaseTestQuery(), + mode: "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT,FLEX,WALK,BIKE" + }) + ).toMatchSnapshot(); + expect( + getRoutingParams(fakeConfig, { + ...makeBaseTestQuery(), + mode: + "FLEX_EGRESS,FLEX,FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT,FLEX,WALK,BIKE,WALK,TRANSIT,BUS" + }) + ).toMatchSnapshot(); + }); }); describe("isNotDefaultQuery", () => { diff --git a/packages/core-utils/src/query.js b/packages/core-utils/src/query.js index cb5801ae9..a553f47ee 100644 --- a/packages/core-utils/src/query.js +++ b/packages/core-utils/src/query.js @@ -182,6 +182,28 @@ export function reduceOtpFlexModes(modes) { }, []); } +/** + * Helper method to process a mode string, replacing all instances of FLEX + * with the full set of FLEX modes used by otp-2 + * @param {*} mode a mode String, not an array + * @returns a mode String, not an array (with flex modes expanded) + */ +export function expandOtpFlexMode(mode) { + const modes = reduceOtpFlexModes(mode.split(",")); + return modes + .map(m => { + // If both the expanded and shrunk modes are included, remove the exapnded one + if (m === "FLEX_EGRESS" || m === "FLEX_ACCESS" || m === "FLEX_DIRECT") { + if (mode.includes("FLEX")) return ""; + } + if (m === "FLEX") { + return "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT"; + } + return m; + }) + .join(","); +} + /** * Determines whether the specified query differs from the default query, i.e., * whether the user has modified any trip options (including mode) from their @@ -466,19 +488,8 @@ export function getRoutingParams(config, currentQuery, ignoreRealtimeUpdates) { // Replace FLEX placeholder with OTP flex modes if (params.mode) { // Ensure query is in reduced format to avoid replacing twice - const mode = reduceOtpFlexModes(params.mode.split(",")); - params.mode = mode - .map(m => { - // If both the expanded and shrunk modes are included, remove the exapnded one - if (m === "FLEX_EGRESS" || m === "FLEX_ACCESS" || m === "FLEX_DIRECT") { - if (mode.includes("FLEX")) return ""; - } - if (m === "FLEX") { - return "FLEX_EGRESS,FLEX_ACCESS,FLEX_DIRECT"; - } - return m; - }) - .join(","); + const reducedMode = reduceOtpFlexModes(params.mode.split(",")); + params.mode = expandOtpFlexMode(reducedMode.join(",")); } return params; From d942adf911c22009318496f5c71facfcd836ca84 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Tue, 25 Jan 2022 09:24:37 -0500 Subject: [PATCH 3/4] refactor: reduce commit diff --- packages/core-utils/src/query.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core-utils/src/query.js b/packages/core-utils/src/query.js index a553f47ee..9fe6e41dc 100644 --- a/packages/core-utils/src/query.js +++ b/packages/core-utils/src/query.js @@ -488,8 +488,8 @@ export function getRoutingParams(config, currentQuery, ignoreRealtimeUpdates) { // Replace FLEX placeholder with OTP flex modes if (params.mode) { // Ensure query is in reduced format to avoid replacing twice - const reducedMode = reduceOtpFlexModes(params.mode.split(",")); - params.mode = expandOtpFlexMode(reducedMode.join(",")); + const reducedMode = reduceOtpFlexModes(params.mode.split(",")).join(","); + params.mode = expandOtpFlexMode(reducedMode); } return params; From 7be6cdf4055042904e0d895d22825d5df980920f Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 26 Jan 2022 11:42:42 -0500 Subject: [PATCH 4/4] test: move snapshots to test definition --- .../__snapshots__/query-params.js.snap | 47 ------------------- .../core-utils/src/__tests__/query-params.js | 16 ++++--- 2 files changed, 10 insertions(+), 53 deletions(-) diff --git a/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap b/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap index 05de93dda..4863691a9 100644 --- a/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap +++ b/packages/core-utils/src/__tests__/__snapshots__/query-params.js.snap @@ -1,52 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`flex-reducer should modify a query that includes all flex modes 1`] = ` -Array [ - "WALK", - "TRANSIT", - "BIKE", - "FLEX", -] -`; - -exports[`flex-reducer should modify a query that includes all flex modes 2`] = ` -Array [ - "FLEX", - "BIKE", - "WALK", - "TRANSIT", -] -`; - -exports[`flex-reducer should modify a query that includes duplicate flex modes 1`] = ` -Array [ - "FLEX", -] -`; - -exports[`flex-reducer should modify a query that includes only flex modes 1`] = ` -Array [ - "FLEX", -] -`; - -exports[`flex-reducer should modify a query that includes some flex modes 1`] = ` -Array [ - "WALK", - "TRANSIT", - "BIKE", - "FLEX", -] -`; - -exports[`flex-reducer should not touch a query that doesn't include flex modes 1`] = ` -Array [ - "WALK", - "TRANSIT", - "BIKE", -] -`; - exports[`query-params getCustomQueryParams should return queryParams with customizations 1`] = ` Array [ Object { diff --git a/packages/core-utils/src/__tests__/query-params.js b/packages/core-utils/src/__tests__/query-params.js index afecc3439..3be578a84 100644 --- a/packages/core-utils/src/__tests__/query-params.js +++ b/packages/core-utils/src/__tests__/query-params.js @@ -42,12 +42,16 @@ describe("query-params", () => { describe("flex-reducer", () => { it("should not touch a query that doesn't include flex modes", () => { - expect(reduceOtpFlexModes(["WALK", "TRANSIT", "BIKE"])).toMatchSnapshot(); + expect(reduceOtpFlexModes(["WALK", "TRANSIT", "BIKE"])).toStrictEqual([ + "WALK", + "TRANSIT", + "BIKE" + ]); }); it("should modify a query that includes some flex modes", () => { expect( reduceOtpFlexModes(["WALK", "TRANSIT", "BIKE", "FLEX_DIRECT"]) - ).toMatchSnapshot(); + ).toStrictEqual(["WALK", "TRANSIT", "BIKE", "FLEX"]); }); it("should modify a query that includes all flex modes", () => { expect( @@ -59,7 +63,7 @@ describe("flex-reducer", () => { "FLEX_ACCESS", "FLEX_EGRESS" ]) - ).toMatchSnapshot(); + ).toStrictEqual(["WALK", "TRANSIT", "BIKE", "FLEX"]); expect( reduceOtpFlexModes([ "FLEX_DIRECT", @@ -69,12 +73,12 @@ describe("flex-reducer", () => { "FLEX_EGRESS", "TRANSIT" ]) - ).toMatchSnapshot(); + ).toStrictEqual(["FLEX", "BIKE", "WALK", "TRANSIT"]); }); it("should modify a query that includes only flex modes", () => { expect( reduceOtpFlexModes(["FLEX_DIRECT", "FLEX_ACCESS", "FLEX_EGRESS"]) - ).toMatchSnapshot(); + ).toStrictEqual(["FLEX"]); }); it("should modify a query that includes duplicate flex modes", () => { expect( @@ -84,6 +88,6 @@ describe("flex-reducer", () => { "FLEX_ACCESS", "FLEX_EGRESS" ]) - ).toMatchSnapshot(); + ).toStrictEqual(["FLEX"]); }); });