Skip to content

Commit c668101

Browse files
marcelklehrmarcelklehr
committed
fix(failsafe): Also apply failsafe on upstream changes
Signed-off-by: Marcel Klehr <[email protected]>
1 parent 7636360 commit c668101

File tree

3 files changed

+126
-17
lines changed

3 files changed

+126
-17
lines changed

‎src/lib/strategies/Default.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,10 @@ export default class SyncProcess {
246246

247247
Logger.log({localPlan: this.localPlanStage2, serverPlan: this.serverPlanStage2})
248248

249-
this.applyDeletionFailsafe(this.localPlanStage2.REMOVE)
250-
this.applyAdditionFailsafe(this.localPlanStage2.CREATE)
249+
this.applyDeletionFailsafe(this.localTreeRoot, this.localPlanStage2.REMOVE)
250+
this.applyAdditionFailsafe(this.localTreeRoot, this.localPlanStage2.CREATE)
251+
this.applyDeletionFailsafe(this.serverTreeRoot, this.serverPlanStage2.REMOVE)
252+
this.applyAdditionFailsafe(this.serverTreeRoot, this.serverPlanStage2.CREATE)
251253

252254
if (!this.localDonePlan) {
253255
this.localDonePlan = {
@@ -361,30 +363,30 @@ export default class SyncProcess {
361363
this.serverTreeRoot.createIndex()
362364
}
363365

364-
protected applyDeletionFailsafe(removals: Diff<TItemLocation, TItemLocation, RemoveAction<TItemLocation, TItemLocation>>) {
365-
const localCountTotal = this.localTreeRoot.count()
366-
const localCountDeleted = removals.getActions().reduce((count, action) => count + action.payload.count(), 0)
366+
protected applyDeletionFailsafe(tree: Folder<TItemLocation>, removals: Diff<TItemLocation, TItemLocation, RemoveAction<TItemLocation, TItemLocation>>) {
367+
const countTotal = tree.count()
368+
const countDeleted = removals.getActions().reduce((count, action) => count + action.payload.count(), 0)
367369

368-
Logger.log('Checking deletion failsafe: ' + localCountDeleted + '/' + localCountTotal + '=' + (localCountDeleted / localCountTotal))
370+
Logger.log('Checking deletion failsafe: ' + countDeleted + '/' + countTotal + '=' + (countDeleted / countTotal))
369371
// Failsafe kicks in if more than 20% is deleted or more than 1k bookmarks
370-
if ((localCountTotal > 5 && localCountDeleted / localCountTotal > 0.2) || localCountDeleted > 1000) {
372+
if ((countTotal > 5 && countDeleted / countTotal > 0.2) || countDeleted > 1000) {
371373
const failsafe = this.server.getData().failsafe
372374
if (failsafe !== false || typeof failsafe === 'undefined') {
373-
throw new DeletionFailsafeError(Math.ceil((localCountDeleted / localCountTotal) * 100))
375+
throw new DeletionFailsafeError(Math.ceil((countDeleted / countTotal) * 100))
374376
}
375377
}
376378
}
377379

378-
protected applyAdditionFailsafe(creations: Diff<TItemLocation, TItemLocation, CreateAction<TItemLocation, TItemLocation>>) {
379-
const localCountTotal = this.localTreeRoot.count()
380-
const localCountAdded = creations.getActions().reduce((count, action) => count + action.payload.count(), 0)
380+
protected applyAdditionFailsafe(tree: Folder<TItemLocation>, creations: Diff<TItemLocation, TItemLocation, CreateAction<TItemLocation, TItemLocation>>) {
381+
const countTotal = tree.count()
382+
const countAdded = creations.getActions().reduce((count, action) => count + action.payload.count(), 0)
381383

382-
Logger.log('Checking addition failsafe: ' + localCountAdded + '/' + localCountTotal + '=' + (localCountAdded / localCountTotal))
384+
Logger.log('Checking addition failsafe: ' + countAdded + '/' + countTotal + '=' + (countAdded / countTotal))
383385
// Failsafe kicks in if more than 20% is added or more than 1k bookmarks
384-
if (localCountTotal > 5 && (localCountAdded / localCountTotal > 0.2 || localCountAdded > 1000)) {
386+
if (countTotal > 5 && (countAdded / countTotal > 0.2 || countAdded > 1000)) {
385387
const failsafe = this.server.getData().failsafe
386388
if (failsafe !== false || typeof failsafe === 'undefined') {
387-
throw new AdditionFailsafeError(Math.ceil((localCountAdded / localCountTotal) * 100))
389+
throw new AdditionFailsafeError(Math.ceil((countAdded / countTotal) * 100))
388390
}
389391
}
390392
}

‎src/lib/strategies/Unidirectional.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,11 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy {
158158
this.actionsPlanned = Object.values(this.revertPlan).reduce((acc, diff) => diff.getActions().length + acc, 0)
159159

160160
if (this.direction === ItemLocation.LOCAL) {
161-
this.applyDeletionFailsafe(this.revertPlan.REMOVE)
162-
this.applyAdditionFailsafe(this.revertPlan.CREATE)
161+
this.applyDeletionFailsafe(this.localTreeRoot, this.revertPlan.REMOVE)
162+
this.applyAdditionFailsafe(this.localTreeRoot, this.revertPlan.CREATE)
163+
} else {
164+
this.applyDeletionFailsafe(this.serverTreeRoot, this.revertPlan.REMOVE)
165+
this.applyAdditionFailsafe(this.serverTreeRoot, this.revertPlan.CREATE)
163166
}
164167

165168
if (this.canceled) {

‎src/test/test.js‎

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2189,7 +2189,7 @@ describe('Floccus', function() {
21892189
title: 'bar',
21902190
parentId: fooFolder.id
21912191
})
2192-
// We need more than 5 bookmarks to be above the neglibility threshold
2192+
// We need more than 5 bookmarks to be above the negligibility threshold
21932193
await Promise.all([
21942194
'http://ur.l/',
21952195
'http://ur.ll/',
@@ -2291,6 +2291,110 @@ describe('Floccus', function() {
22912291
expect(account.getData().error).to.be.ok // should have errored
22922292
expect(account.getData().error).to.include((new AdditionFailsafeError).code)
22932293
})
2294+
it('should error when deleting too much remote data (failsafe)', async function() {
2295+
if (ACCOUNT_DATA.noCache) {
2296+
this.skip()
2297+
}
2298+
2299+
const localRoot = account.getData().localRoot
2300+
const fooFolder = await browser.bookmarks.create({
2301+
title: 'foo',
2302+
parentId: localRoot
2303+
})
2304+
const barFolder = await browser.bookmarks.create({
2305+
title: 'bar',
2306+
parentId: fooFolder.id
2307+
})
2308+
// We need more than 5 bookmarks to be above the negligibility threshold
2309+
await Promise.all([
2310+
'http://ur.l/',
2311+
'http://ur.ll/',
2312+
'http://ur2.l/',
2313+
'http://ur3.l/',
2314+
'http://ur4.l/',
2315+
'http://ur5.l/',
2316+
'http://ur6.l/',
2317+
'http://ur7.l/',
2318+
'http://ur8.l/',
2319+
'http://ur9.l/',
2320+
'http://ur10.l/',
2321+
].map(url => browser.bookmarks.create({
2322+
title: 'url',
2323+
url,
2324+
parentId: barFolder.id
2325+
})))
2326+
await account.sync()
2327+
expect(account.getData().error).to.not.be.ok
2328+
2329+
// Remove everything
2330+
await browser.bookmarks.removeTree(barFolder.id)
2331+
2332+
await account.sync()
2333+
expect(account.getData().error).to.be.ok // should have errored
2334+
expect(account.getData().error).to.include((new DeletionFailsafeError).code)
2335+
})
2336+
it('should error when adding too much remote data (failsafe)', async function() {
2337+
if (ACCOUNT_DATA.noCache) {
2338+
this.skip()
2339+
}
2340+
2341+
const localRoot = account.getData().localRoot
2342+
const fooFolder = await browser.bookmarks.create({
2343+
title: 'foo',
2344+
parentId: localRoot
2345+
})
2346+
const barFolder = await browser.bookmarks.create({
2347+
title: 'bar',
2348+
parentId: fooFolder.id
2349+
})
2350+
// Create 6 bookmarks to be above the negligibility threshold
2351+
await Promise.all([
2352+
'http://ur.1l/',
2353+
'http://ur2.l/',
2354+
'http://ur3.l/',
2355+
'http://ur4.l/',
2356+
'http://ur5.l/',
2357+
'http://ur6.l/'
2358+
].map(url => browser.bookmarks.create({
2359+
title: 'url',
2360+
url,
2361+
parentId: barFolder.id
2362+
})))
2363+
await account.sync()
2364+
expect(account.getData().error).to.not.be.ok
2365+
2366+
// Simulate an extreme increase of bookmarks
2367+
await Promise.all([
2368+
'http://ur.1l/',
2369+
'http://ur2.l/',
2370+
'http://ur3.l/',
2371+
'http://ur4.l/',
2372+
'http://ur5.l/',
2373+
'http://ur6.l/',
2374+
'http://ur7.l/',
2375+
'http://ur8.l/',
2376+
'http://ur9.l/',
2377+
'http://ur10.l/',
2378+
'http://ur.11l/',
2379+
'http://ur12.l/',
2380+
'http://ur13.l/',
2381+
'http://ur14.l/',
2382+
'http://ur15.l/',
2383+
'http://ur16.l/',
2384+
'http://ur17.l/',
2385+
'http://ur18.l/',
2386+
'http://ur19.l/',
2387+
'http://ur20.l/',
2388+
].map(url => browser.bookmarks.create({
2389+
title: 'url',
2390+
url,
2391+
parentId: barFolder.id
2392+
})))
2393+
2394+
await account.sync()
2395+
expect(account.getData().error).to.be.ok // should have errored
2396+
expect(account.getData().error).to.include((new AdditionFailsafeError).code)
2397+
})
22942398
it('should leave alone unaccepted bookmarks entirely', async function() {
22952399
if (!~ACCOUNT_DATA.type.indexOf('nextcloud')) {
22962400
return this.skip()

0 commit comments

Comments
 (0)