From c927be7a0dee71ccde5acd565579d65cbd6374b1 Mon Sep 17 00:00:00 2001 From: Ben West Date: Mon, 20 Apr 2026 17:49:23 -0700 Subject: [PATCH] Revert "fix(api3): dedup AAPS profile-store edits via REST POST /v3/profile" This reverts commit 3b786ab3. On review the V3 (app, defaultProfile) collapse was too aggressive and broke parity with how the rest of the ecosystem treats the profile collection: - Loop (NightscoutKit) and Trio (NightscoutAPI.swift:411) both POST /api/v1/profile without _id on every settings edit, accumulating one doc per upload via lib/server/profile.js:create(). They have done so for years. - The Nightscout profile collection is historical/append-only by design; the NS UI profile editor lets users navigate prior snapshots, and lib/server/profile.js:last() picks the most recent for display. - Collapsing AAPS V3 edits onto a single (app, defaultProfile) row diverged from Loop/Trio/AAPS-V1 behavior and erased the upload history that NS UI exposes. The original 'AAPS edits not appearing' user complaint is sufficiently addressed by: - the V1 websocket retry dedup (commit 85f7e6ac), which kills the 60s ack-window race; and - the {startDate: -1, _id: -1} secondary sort in profile.last() (also 85f7e6ac), which deterministically picks the newest row when startDate ties. Both of those help every uploader (Loop, Trio, AAPS V1, AAPS V3) without changing the ecosystem-wide profile-as-history semantic. The characterization tests added in ddabdc6c are restored by this revert and continue to document V3's request-level (date-based) dedup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/api3/generic/update/validate.js | 12 -------- lib/api3/shared/operationTools.js | 11 ------- tests/api3.aaps-patterns.test.js | 45 +++++------------------------ 3 files changed, 8 insertions(+), 60 deletions(-) diff --git a/lib/api3/generic/update/validate.js b/lib/api3/generic/update/validate.js index f911f76d..65233ddd 100644 --- a/lib/api3/generic/update/validate.js +++ b/lib/api3/generic/update/validate.js @@ -21,15 +21,6 @@ function validate (opCtx, doc, storageDoc, options) { const immutable = ['identifier', 'date', 'utcOffset', 'eventType', 'device', 'app', 'srvCreated', 'subject', 'srvModified', 'modifiedBy', 'isValid']; - // Profile-store documents are dedup'd by (app, defaultProfile) — the whole - // point of that dedup is that AAPS-style edits carry a NEW `date` - // (LocalProfileLastChange) but should still update the existing row. - // Relax `date`/`created_at`/`startDate` immutability for profile-store - // deduplication so the latest edit overwrites the previous snapshot. - const isProfileStoreDedup = isDeduplication - && storageDoc && storageDoc.defaultProfile && storageDoc.store; - const profileStoreMutable = new Set(['date', 'created_at', 'startDate']); - if (storageDoc.isReadOnly === true || storageDoc.readOnly === true || storageDoc.readonly === true) { return opTools.sendJSONStatus(res, apiConst.HTTP.UNPROCESSABLE_ENTITY, apiConst.MSG.HTTP_422_READONLY_MODIFICATION); @@ -45,9 +36,6 @@ function validate (opCtx, doc, storageDoc, options) { if (storageDoc.isValid === false) continue; - if (isProfileStoreDedup && profileStoreMutable.has(field)) - continue; - if (typeof(doc[field]) !== 'undefined' && doc[field] !== storageDoc[field]) { return opTools.sendJSONStatus(res, apiConst.HTTP.BAD_REQUEST, apiConst.MSG.HTTP_400_IMMUTABLE_FIELD.replace('{0}', field)); diff --git a/lib/api3/shared/operationTools.js b/lib/api3/shared/operationTools.js index 393913b6..c8b8a008 100644 --- a/lib/api3/shared/operationTools.js +++ b/lib/api3/shared/operationTools.js @@ -98,17 +98,6 @@ function calculateIdentifier (doc) { if (!doc) return undefined; - // Profile-store documents are a singleton-per-(app, defaultProfile) by design: - // each source (e.g. AAPS) has one "current profile store" at a time. Without - // this special case, every edit produces a new identifier (because `date` - // changes per edit and `device`/`eventType` are absent) and accumulates - // duplicate profile documents. Dedup by (app, defaultProfile) so that - // re-sends and edits collapse onto the same row. - if (doc.defaultProfile && doc.store && !doc.eventType) { - const profileKey = 'profilestore_' + (doc.app || 'unknown') + '_' + doc.defaultProfile; - return uuid.v5(profileKey, uuidNamespace); - } - let key = doc.device + '_' + doc.date; if (doc.eventType) { key += '_' + doc.eventType; diff --git a/tests/api3.aaps-patterns.test.js b/tests/api3.aaps-patterns.test.js index db5409b8..228b2f77 100644 --- a/tests/api3.aaps-patterns.test.js +++ b/tests/api3.aaps-patterns.test.js @@ -652,11 +652,8 @@ describe('API3 AAPS Patterns - Deduplication and Real-world Scenarios', function docs.length.should.equal(1); }); - it('AAPS edit (new date from LocalProfileLastChange) DEDUPS onto existing profile (post-fix)', async () => { - // Simulates: user edits profile in AAPS twice -> two distinct LocalProfileLastChange values. - // Pre-fix: V3 inserted a new doc per edit because identifier = uuidv5("undefined_"). - // Post-fix: profile-store identifier is uuidv5("profilestore__"), - // so edits with the same (app, defaultProfile) replace the existing doc. + it('AAPS edit (new date from LocalProfileLastChange) creates a SECOND doc, not an update', async () => { + // Simulates: user edits profile in AAPS twice -> two distinct LocalProfileLastChange values const t1 = Date.now() - 60000; const first = aapsV3Profile(t1); first.store['aaps-v3-test'].carbratio[0].value = 8; @@ -667,16 +664,15 @@ describe('API3 AAPS Patterns - Deduplication and Real-world Scenarios', function res1.status.should.equal(201); self.cache.clear(); - const res2 = await self.instance.post(url, self.jwt.update).send(second); - // Post-fix: edit dedups in place -> 200, same identifier - res2.status.should.equal(200); - res2.body.identifier.should.equal(res1.body.identifier); + const res2 = await self.instance.post(url, self.jwt.create).send(second); + // V3 inserts a NEW doc because identifier (uuidv5 of "undefined_") differs + res2.status.should.equal(201); + res2.body.identifier.should.not.equal(res1.body.identifier); const docs = await profileCollection().find({ defaultProfile: 'aaps-v3-test' }).toArray(); - docs.length.should.equal(1); - docs[0].store['aaps-v3-test'].carbratio[0].value.should.equal(14); + docs.length.should.equal(2); - // Verify ctx.profile.last() returns the updated profile (post-fix sort: startDate desc, _id desc) + // Verify ctx.profile.last() returns the newer profile (post-fix sort: startDate desc, _id desc) await new Promise((resolve, reject) => { self.instance.ctx.profile.last((err, lastDocs) => { if (err) return reject(err); @@ -688,30 +684,5 @@ describe('API3 AAPS Patterns - Deduplication and Real-world Scenarios', function }); }); }); - - it('different defaultProfile names produce distinct V3 profile docs', async () => { - // Two different profile names should NOT collide under the new dedup key. - const a = aapsV3Profile(Date.now()); - a.defaultProfile = 'aaps-v3-test'; - a.store = { 'aaps-v3-test': a.store['aaps-v3-test'] }; - const b = aapsV3Profile(Date.now() + 1); - b.defaultProfile = 'aaps-v3-test-other'; - b.store = { 'aaps-v3-test-other': a.store['aaps-v3-test'] }; - - const resA = await self.instance.post(url, self.jwt.create).send(a); - resA.status.should.equal(201); - self.cache.clear(); - const resB = await self.instance.post(url, self.jwt.create).send(b); - resB.status.should.equal(201); - resB.body.identifier.should.not.equal(resA.body.identifier); - - const docsA = await profileCollection().find({ defaultProfile: 'aaps-v3-test' }).toArray(); - const docsB = await profileCollection().find({ defaultProfile: 'aaps-v3-test-other' }).toArray(); - docsA.length.should.equal(1); - docsB.length.should.equal(1); - - // cleanup the extra one - await profileCollection().deleteMany({ defaultProfile: 'aaps-v3-test-other' }); - }); }); });