mirror of
https://github.com/bckelley/cgm-remote-monitor.git
synced 2026-08-24 03:14:12 -05:00
Revert "fix(api3): dedup AAPS profile-store edits via REST POST /v3/profile"
This reverts commit3b786ab3. 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 (commit85f7e6ac), which kills the 60s ack-window race; and - the {startDate: -1, _id: -1} secondary sort in profile.last() (also85f7e6ac), 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 inddabdc6care 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>
This commit is contained in:
@@ -21,15 +21,6 @@ function validate (opCtx, doc, storageDoc, options) {
|
|||||||
const immutable = ['identifier', 'date', 'utcOffset', 'eventType', 'device', 'app',
|
const immutable = ['identifier', 'date', 'utcOffset', 'eventType', 'device', 'app',
|
||||||
'srvCreated', 'subject', 'srvModified', 'modifiedBy', 'isValid'];
|
'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) {
|
if (storageDoc.isReadOnly === true || storageDoc.readOnly === true || storageDoc.readonly === true) {
|
||||||
return opTools.sendJSONStatus(res, apiConst.HTTP.UNPROCESSABLE_ENTITY,
|
return opTools.sendJSONStatus(res, apiConst.HTTP.UNPROCESSABLE_ENTITY,
|
||||||
apiConst.MSG.HTTP_422_READONLY_MODIFICATION);
|
apiConst.MSG.HTTP_422_READONLY_MODIFICATION);
|
||||||
@@ -45,9 +36,6 @@ function validate (opCtx, doc, storageDoc, options) {
|
|||||||
if (storageDoc.isValid === false)
|
if (storageDoc.isValid === false)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
if (isProfileStoreDedup && profileStoreMutable.has(field))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
if (typeof(doc[field]) !== 'undefined' && doc[field] !== storageDoc[field]) {
|
if (typeof(doc[field]) !== 'undefined' && doc[field] !== storageDoc[field]) {
|
||||||
return opTools.sendJSONStatus(res, apiConst.HTTP.BAD_REQUEST,
|
return opTools.sendJSONStatus(res, apiConst.HTTP.BAD_REQUEST,
|
||||||
apiConst.MSG.HTTP_400_IMMUTABLE_FIELD.replace('{0}', field));
|
apiConst.MSG.HTTP_400_IMMUTABLE_FIELD.replace('{0}', field));
|
||||||
|
|||||||
@@ -98,17 +98,6 @@ function calculateIdentifier (doc) {
|
|||||||
if (!doc)
|
if (!doc)
|
||||||
return undefined;
|
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;
|
let key = doc.device + '_' + doc.date;
|
||||||
if (doc.eventType) {
|
if (doc.eventType) {
|
||||||
key += '_' + doc.eventType;
|
key += '_' + doc.eventType;
|
||||||
|
|||||||
@@ -652,11 +652,8 @@ describe('API3 AAPS Patterns - Deduplication and Real-world Scenarios', function
|
|||||||
docs.length.should.equal(1);
|
docs.length.should.equal(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('AAPS edit (new date from LocalProfileLastChange) DEDUPS onto existing profile (post-fix)', async () => {
|
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.
|
// Simulates: user edits profile in AAPS twice -> two distinct LocalProfileLastChange values
|
||||||
// Pre-fix: V3 inserted a new doc per edit because identifier = uuidv5("undefined_<date>").
|
|
||||||
// Post-fix: profile-store identifier is uuidv5("profilestore_<app>_<defaultProfile>"),
|
|
||||||
// so edits with the same (app, defaultProfile) replace the existing doc.
|
|
||||||
const t1 = Date.now() - 60000;
|
const t1 = Date.now() - 60000;
|
||||||
const first = aapsV3Profile(t1);
|
const first = aapsV3Profile(t1);
|
||||||
first.store['aaps-v3-test'].carbratio[0].value = 8;
|
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);
|
res1.status.should.equal(201);
|
||||||
self.cache.clear();
|
self.cache.clear();
|
||||||
|
|
||||||
const res2 = await self.instance.post(url, self.jwt.update).send(second);
|
const res2 = await self.instance.post(url, self.jwt.create).send(second);
|
||||||
// Post-fix: edit dedups in place -> 200, same identifier
|
// V3 inserts a NEW doc because identifier (uuidv5 of "undefined_<date>") differs
|
||||||
res2.status.should.equal(200);
|
res2.status.should.equal(201);
|
||||||
res2.body.identifier.should.equal(res1.body.identifier);
|
res2.body.identifier.should.not.equal(res1.body.identifier);
|
||||||
|
|
||||||
const docs = await profileCollection().find({ defaultProfile: 'aaps-v3-test' }).toArray();
|
const docs = await profileCollection().find({ defaultProfile: 'aaps-v3-test' }).toArray();
|
||||||
docs.length.should.equal(1);
|
docs.length.should.equal(2);
|
||||||
docs[0].store['aaps-v3-test'].carbratio[0].value.should.equal(14);
|
|
||||||
|
|
||||||
// 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) => {
|
await new Promise((resolve, reject) => {
|
||||||
self.instance.ctx.profile.last((err, lastDocs) => {
|
self.instance.ctx.profile.last((err, lastDocs) => {
|
||||||
if (err) return reject(err);
|
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' });
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user