From 1a0d7a0aca3a77002be136cc00842c46e1ad4925 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 17 Dec 2024 23:14:34 -0700 Subject: [PATCH] Remove Interval Turnover --- .../rules-engine/src/provider-wireup.js | 83 ++++------- .../rules-engine/test/integration.spec.js | 77 +--------- .../rules-engine/test/provider-wireup.spec.js | 136 +----------------- 3 files changed, 33 insertions(+), 263 deletions(-) diff --git a/shared-libs/rules-engine/src/provider-wireup.js b/shared-libs/rules-engine/src/provider-wireup.js index 09812430b0..3747e5233f 100644 --- a/shared-libs/rules-engine/src/provider-wireup.js +++ b/shared-libs/rules-engine/src/provider-wireup.js @@ -12,7 +12,6 @@ const refreshRulesEmissions = require('./refresh-rules-emissions'); const rulesEmitter = require('./rules-emitter'); const rulesStateStore = require('./rules-state-store'); const updateTemporalStates = require('./update-temporal-states'); -const calendarInterval = require('@medic/calendar-interval'); let wireupOptions; @@ -31,7 +30,7 @@ module.exports = { * @param {number} settings.monthStartDate reporting interval start date * @param {Object} userDoc User's hydrated contact document */ - initialize: (provider, settings) => { + initialize: async (provider, settings) => { const isEnabled = rulesEmitter.initialize(settings); if (!isEnabled) { return Promise.resolve(); @@ -40,26 +39,21 @@ module.exports = { const { enableTasks=true, enableTargets=true } = settings; wireupOptions = { enableTasks, enableTargets }; - return provider - .existingRulesStateStore() - .then(existingStateDoc => { - if (!rulesEmitter.isLatestNoolsSchema()) { - throw Error('Rules Engine: Updates to the nools schema are required'); - } + const existingStateDoc = await provider.existingRulesStateStore(); + if (!rulesEmitter.isLatestNoolsSchema()) { + throw Error('Rules Engine: Updates to the nools schema are required'); + } - const contactClosure = updatedState => provider.stateChangeCallback( - existingStateDoc, - { rulesStateStore: updatedState } - ); - const needsBuilding = rulesStateStore.load(existingStateDoc.rulesStateStore, settings, contactClosure); - return handleIntervalTurnover(provider, settings).then(() => { - if (!needsBuilding) { - return; - } - - rulesStateStore.build(settings, contactClosure); - }); - }); + const contactClosure = updatedState => provider.stateChangeCallback( + existingStateDoc, + { rulesStateStore: updatedState } + ); + const needsBuilding = rulesStateStore.load(existingStateDoc.rulesStateStore, settings, contactClosure); + if (!needsBuilding) { + return; + } + + rulesStateStore.build(settings, contactClosure); }, /** @@ -273,20 +267,18 @@ const refreshRulesEmissionForContacts = (provider, calculationTimestamp, contact }); }; - return handleIntervalTurnover(provider, { monthStartDate: rulesStateStore.getMonthStartDate() }).then(() => { - if (contactIds) { - return refreshForKnownContacts(calculationTimestamp, contactIds); - } + if (contactIds) { + return refreshForKnownContacts(calculationTimestamp, contactIds); + } - // If the contact state store does not contain all contacts, build up that list (contact doc ids + headless ids in - // reports/tasks) - if (!rulesStateStore.hasAllContacts()) { - return refreshForAllContacts(calculationTimestamp); - } + // If the contact state store does not contain all contacts, build up that list (contact doc ids + headless ids in + // reports/tasks) + if (!rulesStateStore.hasAllContacts()) { + return refreshForAllContacts(calculationTimestamp); + } - // Once the contact state store has all contacts, trust it and only refresh those marked dirty - return refreshForKnownContacts(calculationTimestamp, rulesStateStore.getContactIds()); - }); + // Once the contact state store has all contacts, trust it and only refresh those marked dirty + return refreshForKnownContacts(calculationTimestamp, rulesStateStore.getContactIds()); }; const storeTargetsDoc = (provider, aggregate, updatedTargets) => { @@ -305,28 +297,3 @@ const storeTargetsDoc = (provider, aggregate, updatedTargets) => { ); }; -// This function takes the last saved state (which may be stale) and generates the targets doc for the corresponding -// reporting interval (that includes the date when the state was calculated). -// We don't recalculate the state prior to this because we support targets that count events infinitely to emit `now`, -// which means that they would all be excluded from the emission filter (being outside the past reporting interval). -// https://github.com/medic/cht-core/issues/6209 -const handleIntervalTurnover = async (provider, { monthStartDate }) => { - if (!rulesStateStore.isLoaded() || !wireupOptions.enableTargets) { - return Promise.resolve(); - } - - const stateCalculatedAt = rulesStateStore.stateLastUpdatedAt(); - if (!stateCalculatedAt) { - return Promise.resolve(); - } - - const currentInterval = calendarInterval.getCurrent(monthStartDate); - // 4th parameter of isBetween represents inclusivity. By default or using ( is exclusive, [ is inclusive - if (moment(stateCalculatedAt).isBetween(currentInterval.start, currentInterval.end, null, '[]')) { - return Promise.resolve(); - } - - const filterInterval = calendarInterval.getInterval(monthStartDate, stateCalculatedAt); - const aggregate = await rulesStateStore.getTargetAggregates(filterInterval); - return storeTargetsDoc(provider, aggregate, true); -}; diff --git a/shared-libs/rules-engine/test/integration.spec.js b/shared-libs/rules-engine/test/integration.spec.js index 155bbbf53f..d6d4874c64 100644 --- a/shared-libs/rules-engine/test/integration.spec.js +++ b/shared-libs/rules-engine/test/integration.spec.js @@ -9,7 +9,6 @@ const sinon = require('sinon'); const RulesEngine = require('../src'); const rulesEmitter = require('../src/rules-emitter'); -const calendarInterval = require('@medic/calendar-interval'); const { expect } = chai; chai.use(chaiExclude); @@ -259,18 +258,10 @@ describe(`Rules Engine Integration Tests`, () => { clock.setSystemTime(TEST_START + MS_IN_DAY * 39); const monthLater = await rulesEngine.fetchTasksFor(['patient']); expect(monthLater).to.have.property('length', 0); - expect(db.bulkDocs.callCount).to.eq(5); - - // interval turnover - expect(db.bulkDocs.args[3][0].docs[0]).to.deep.include({ - _id: `target~${TARGET_INTERVAL}~user~org.couchdb.user:username`, - type: 'target', - owner: 'user', - reporting_period: TARGET_INTERVAL, - }); + expect(db.bulkDocs.callCount).to.eq(4); const dateNext = moment(TEST_START + MS_IN_DAY * 39).format('YYYY-MM'); - expect(db.bulkDocs.args[4][0].docs[0]).to.deep.include({ + expect(db.bulkDocs.args[3][0].docs[0]).to.deep.include({ _id: `target~${dateNext}~user~org.couchdb.user:username`, type: 'target', owner: 'user', @@ -692,70 +683,6 @@ describe(`Rules Engine Integration Tests`, () => { pass: 1, }); }); - - it('targets on interval turnover only recalculates targets when interval changes', async () => { - const targetsSaved = () => { - const targets = []; - db.bulkDocs.args.forEach(([docs]) => { - if (!docs) { - return; - } - - if (docs && docs.docs) { - docs = docs.docs; - } - docs.forEach(doc => doc._id.startsWith('target') && targets.push(doc)); - }); - - return targets; - }; - - clock.setSystemTime(TEST_START); - const patientContact2 = Object.assign({}, patientContact, { _id: 'patient2', patient_id: 'patient_id2', }); - const pregnancyRegistrationReport2 = Object.assign( - {}, - pregnancyRegistrationReport, - { - _id: 'pregReg2', - fields: { lmp_date_8601: TEST_START, patient_id: patientContact2.patient_id }, - reported_date: TEST_START+1 - }, - ); - await db.bulkDocs([patientContact, patientContact2, pregnancyRegistrationReport, pregnancyRegistrationReport2]); - await rulesEngine.updateEmissionsFor(['patient']); - // we're in THE_FUTURE and our state is fresh - - sinon.spy(db, 'bulkDocs'); - sinon.spy(db, 'query'); - const targets = await fetchTargets(); - expect(db.query.callCount).to.eq(expectedQueriesForAllFreshData.length); - expect(targets[['pregnancy-registrations-this-month']].value).to.deep.eq({ - total: 2, - pass: 2, - }); - expect(targetsSaved().length).to.equal(1); - - const sameTargets = await fetchTargets(); - expect(db.query.callCount).to.eq(expectedQueriesForAllFreshData.length); - expect(sameTargets).to.deep.eq(targets); - expect(targetsSaved().length).to.equal(1); - - // fast forward one month - clock.tick(moment(TEST_START).add(1, 'month').diff(moment(TEST_START)) + 2); - const newTargets = await fetchTargets(calendarInterval.getCurrent()); - expect(newTargets[['pregnancy-registrations-this-month']].value).to.deep.eq({ - total: 0, - pass: 0, - }); - const savedTargets = targetsSaved(); - expect(savedTargets.length).to.equal(3); - - const firstTargetInterval = calendarInterval.getInterval(1, TEST_START); - const secondTargetInterval = calendarInterval.getCurrent(); - expect(savedTargets[0].reporting_period).to.equal(moment(firstTargetInterval.end).format('YYYY-MM')); - expect(savedTargets[1].reporting_period).to.equal(moment(firstTargetInterval.end).format('YYYY-MM')); - expect(savedTargets[2].reporting_period).to.equal(moment(secondTargetInterval.end).format('YYYY-MM')); - }); }); } diff --git a/shared-libs/rules-engine/test/provider-wireup.spec.js b/shared-libs/rules-engine/test/provider-wireup.spec.js index 13fa39bf8a..f0afc81dbb 100644 --- a/shared-libs/rules-engine/test/provider-wireup.spec.js +++ b/shared-libs/rules-engine/test/provider-wireup.spec.js @@ -747,7 +747,7 @@ describe('provider-wireup integration tests', () => { expect(provider.commitTargetDoc.callCount).to.equal(0); }); - it('should update the targets doc when the state was calculated outside of the interval', async () => { + it('should not update the targets doc when the state was calculated outside of the interval', async () => { clock.setSystemTime(moment('2020-04-28').valueOf()); const rules = simpleNoolsTemplate(''); const settings = { @@ -779,13 +779,7 @@ describe('provider-wireup integration tests', () => { clock.setSystemTime(moment('2020-05-02').valueOf()); // next interval await wireup.initialize(provider, settings, {}); - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0]).to.deep.equal([ - [{ id: 'uhc', value: { pass: 1, total: 2 } }], - '2020-04', - { userSettingsDoc: { _id: 'org.couchdb.user:username' }, userContactDoc: { _id: 'mock_user_id' }}, - true, - ]); + expect(provider.commitTargetDoc.called).to.be.false; }); it('should work when the settings have been changed', async () => { @@ -824,13 +818,7 @@ describe('provider-wireup integration tests', () => { clock.setSystemTime(moment('2020-04-28').valueOf()); // next interval await wireup.initialize(provider, settings, {}); - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0]).to.deep.equal([ - [{ id: 'uhc', value: { pass: 2, total: 2 } }], - '2020-04', - { userSettingsDoc: { _id: 'org.couchdb.user:username' }, userContactDoc: { _id: 'mock_user_id' } }, - true, - ]); + expect(provider.commitTargetDoc.called).to.be.false; }); it('should work with old format of the rules state store', async () => { @@ -869,13 +857,7 @@ describe('provider-wireup integration tests', () => { clock.setSystemTime(moment('2020-04-28').valueOf()); // next interval await wireup.initialize(provider, settings, {}); - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0]).to.deep.equal([ - [{ id: 'uhc', value: { pass: 2, total: 2 } }], - '2020-04', - { userSettingsDoc: { _id: 'org.couchdb.user:username' }, userContactDoc: { _id: 'mock_user_id' } }, - true, - ]); + expect(provider.commitTargetDoc.called).to.be.false; }); it('should use inclusive operator when comparing dates (left)', async () => { @@ -909,13 +891,7 @@ describe('provider-wireup integration tests', () => { clock.setSystemTime(moment('2020-06-02').valueOf()); // next interval await wireup.initialize(provider, settings, {}); - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0]).to.deep.equal([ - [{ id: 'uhc', value: { pass: 2, total: 2 } }], - '2020-05', - { userSettingsDoc: { _id: 'org.couchdb.user:username' }, userContactDoc: { _id: 'mock_user_id' } }, - true, - ]); + expect(provider.commitTargetDoc.called).to.be.false; }); it('should use inclusive operator when comparing dates (right)', async () => { @@ -950,13 +926,7 @@ describe('provider-wireup integration tests', () => { clock.setSystemTime(moment('2020-06-02').valueOf()); // next interval await wireup.initialize(provider, settings, {}); - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0]).to.deep.equal([ - [{ id: 'uhc', value: { pass: 2, total: 2 } }], - '2020-05', - { userSettingsDoc: { _id: 'org.couchdb.user:username' }, userContactDoc: { _id: 'mock_user_id' } }, - true, - ]); + expect(provider.commitTargetDoc.called).to.be.false; }); }); @@ -995,100 +965,6 @@ describe('provider-wireup integration tests', () => { expect(provider.commitTargetDoc.args[1][1]).to.equal('2020-04'); expect(provider.commitTargetDoc.args[1][0]).to.deep.equal([{ id: 'uhc', value: { pass: 2, total: 2 }}]); }); - - it('should update targets when in new interval when refreshing tasks', async () => { - const rules = simpleNoolsTemplate(''); - const settings = { - rules, - enableTargets: true, - targets: [{ - id: 'uhc', - }], - monthStartDate: 1, - }; - - clock.setSystemTime(moment('2020-04-30 23:00:00').valueOf()); - await wireup.initialize(provider, settings, {}); - - const emissions = [ - mockTargetEmission('uhc', 'doc4', moment('2020-02-23').valueOf(), true), // passes outside interval - mockTargetEmission('uhc', 'doc2', moment('2020-03-29').valueOf(), true), // passes outside interval - mockTargetEmission('uhc', 'doc1', moment('2020-04-12').valueOf(), true), // passes within interval - mockTargetEmission('uhc', 'doc3', moment('2020-04-14').valueOf(), true), // passes within interval - mockTargetEmission('uhc', 'doc5', moment('2020-05-05').valueOf(), true), // passes outside interval - ]; - - const refreshRulesEmissions = sinon.stub().resolves({ targetEmissions: emissions }); - const withMockRefresher = wireup.__with__({ refreshRulesEmissions }); - - expect(provider.commitTargetDoc.callCount).to.equal(0); - await withMockRefresher(() => wireup.fetchTasksFor(provider)); - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0][1]).to.equal('2020-04'); - expect(provider.commitTargetDoc.args[0][0]).to.deep.equal([{ id: 'uhc', value: { pass: 2, total: 2 }}]); - - clock.tick(5 * 60 * 60 * 1000); // 6 hours, it's now 2020-05-01 04:00:00 - await withMockRefresher(() => wireup.fetchTasksFor(provider)); - - expect(provider.commitTargetDoc.callCount).to.equal(3); - expect(provider.commitTargetDoc.args[1][1]).to.equal('2020-04'); - expect(provider.commitTargetDoc.args[1][0]).to.deep.equal([{ id: 'uhc', value: { pass: 2, total: 2 }}]); - expect(provider.commitTargetDoc.args[2][1]).to.equal('2020-05'); - expect(provider.commitTargetDoc.args[2][0]).to.deep.equal([{ id: 'uhc', value: { pass: 1, total: 1 }}]); - }); - - it('should update targets when in new interval when refreshing targets', async () => { - const rules = simpleNoolsTemplate(''); - const settings = { - rules, - enableTargets: true, - targets: [{ - id: 'uhc', - }], - monthStartDate: 1, - }; - - clock.setSystemTime(moment('2020-04-30 23:00:00').valueOf()); - await wireup.initialize(provider, settings, {}); - - const emissionsBefore = [ - mockTargetEmission('uhc', 'doc4', moment('2020-02-23').valueOf(), true), // passes outside interval - mockTargetEmission('uhc', 'doc2', moment('2020-03-29').valueOf(), true), // passes outside interval - mockTargetEmission('uhc', 'doc1', moment('2020-04-12').valueOf(), true), // passes within interval - mockTargetEmission('uhc', 'doc3', moment('2020-04-14').valueOf(), true), // passes within interval - mockTargetEmission('uhc', 'doc5', moment('2020-05-05').valueOf(), true), // passes outside interval - ]; - - // simulate that we have a target with date: now (doc3) and that gets counted in both targets - const emissionsAfter = [ - mockTargetEmission('uhc', 'doc4', moment('2020-02-23').valueOf(), true), // passes outside interval - mockTargetEmission('uhc', 'doc2', moment('2020-03-29').valueOf(), true), // passes outside interval - mockTargetEmission('uhc', 'doc1', moment('2020-04-12').valueOf(), true), // passes within interval - mockTargetEmission('uhc', 'doc3', moment('2020-05-05').valueOf(), true), // passes within interval - mockTargetEmission('uhc', 'doc5', moment('2020-05-05').valueOf(), true), // passes outside interval - ]; - - const refreshRulesEmissions = sinon.stub() - .onCall(0).resolves({ targetEmissions: emissionsBefore }) - .onCall(1).resolves({ targetEmissions: emissionsAfter }); - - const withMockRefresher = wireup.__with__({ refreshRulesEmissions }); - // make sure our state has been "calculated" at least once! - await withMockRefresher(() => wireup.fetchTasksFor(provider)); - - expect(provider.commitTargetDoc.callCount).to.equal(1); - expect(provider.commitTargetDoc.args[0][1]).to.equal('2020-04'); - expect(provider.commitTargetDoc.args[0][0]).to.deep.equal([{ id: 'uhc', value: { pass: 2, total: 2 }}]); - - clock.tick(5 * 60 * 60 * 1000); // 6 hours, it's now 2020-05-01 04:00:00 - await withMockRefresher(() => wireup.fetchTargets(provider)); - - expect(provider.commitTargetDoc.callCount).to.equal(3); - expect(provider.commitTargetDoc.args[1][1]).to.equal('2020-04'); - expect(provider.commitTargetDoc.args[1][0]).to.deep.equal([{ id: 'uhc', value: { pass: 2, total: 2 }}]); - expect(provider.commitTargetDoc.args[2][1]).to.equal('2020-05'); - expect(provider.commitTargetDoc.args[2][0]).to.deep.equal([{ id: 'uhc', value: { pass: 2, total: 2 }}]); - }); }); });