Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to use location types #1687

Merged
merged 24 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
92c9b99
fix(Folder#traverse)
marcelklehr Jun 22, 2024
06b8e74
refactor: Add generic types for item location
marcelklehr Jun 29, 2024
036438e
refactor: Distinguish between plans of different stages and diffs
marcelklehr Jul 27, 2024
ae50c6b
refactor: Get Default SyncProcess in order
marcelklehr Jul 27, 2024
ce1f052
refactor: Refactor Merge and Unidirectional strategies
marcelklehr Jul 28, 2024
dd70104
refactor: Fix Diff#findChain
marcelklehr Jul 28, 2024
d1276a5
refactor: Fix all mentions of TItem and TResource
marcelklehr Jul 28, 2024
61caf6f
Merge branch 'develop' into refactor/location-types
marcelklehr Jul 28, 2024
1a035b1
fix: Folder#clone
marcelklehr Jul 28, 2024
ec567ec
fix: Make default and slave tests pass
marcelklehr Jul 28, 2024
11b9849
fix: Make it pass unidirectional tests
marcelklehr Jul 28, 2024
97f1e73
fix: Make it pass tests on firefox
marcelklehr Jul 28, 2024
c13ed92
fix: Make it pass tests with Nextcloud Bookmarks
marcelklehr Jul 28, 2024
7b5c321
tests: Don't run interrupt tests
marcelklehr Jul 28, 2024
46de945
Merge: Fix full tree loading fallback
marcelklehr Jul 28, 2024
edadef5
Unidirectional: Fix full tree loading fallback
marcelklehr Jul 29, 2024
e90ca33
tests: root folder sync: Don't check order
marcelklehr Jul 29, 2024
1a421b3
fix(Default#executeCreate): Make sure all CREATEs have an oldItem
marcelklehr Jul 29, 2024
29fcd6c
fix(Default#reconcileDiffs): Properly fix type errors
marcelklehr Jul 31, 2024
da982d6
fix(Diff#map): Force parentId mapping for MOVEs
marcelklehr Aug 2, 2024
b33524c
fix(Merge#reconcileDiffs): Clone with location for concurrent Hierarc…
marcelklehr Aug 2, 2024
b5b12b6
fix(Diff#map): Always map payload.parentId to targetLocation
marcelklehr Aug 2, 2024
d2dce29
fix(Diff#map): Don't exclude MOVEs from normal parentId mapping
marcelklehr Aug 2, 2024
4512bd3
fix(bechmark-tests): Add an extra sync-roundtrip
marcelklehr Aug 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/errors/Error.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Bookmark } from '../lib/Tree'
import { Bookmark, TItemLocation } from '../lib/Tree'

export class FloccusError extends Error {
public code: number
Expand Down Expand Up @@ -292,8 +292,8 @@ export class FileUnreadableError extends FloccusError {
}

export class CreateBookmarkError extends FloccusError {
public bookmark: Bookmark
constructor(bookmark: Bookmark) {
public bookmark: Bookmark<TItemLocation>
constructor(bookmark: Bookmark<TItemLocation>) {
super(`E035: Failed to create the following bookmark on the server: ${JSON.stringify(bookmark)}`)
this.code = 35
this.bookmark = bookmark
Expand Down
4 changes: 2 additions & 2 deletions src/lib/Account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export default class Account {
return data
}

async getResource():Promise<OrderFolderResource> {
async getResource():Promise<OrderFolderResource<typeof ItemLocation.LOCAL>> {
return this.localTree
}

Expand Down Expand Up @@ -273,7 +273,7 @@ export default class Account {
// if there is a pending continuation, we resume it

Logger.log('Found existing persisted pending continuation. Resuming last sync')
await this.syncProcess.resumeSync()
await this.syncProcess.sync()
}

await this.setData({ ...this.getData(), scheduled: false, syncing: 1 })
Expand Down
206 changes: 107 additions & 99 deletions src/lib/Diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,68 +14,77 @@ export const ActionType = {

export type TActionType = (typeof ActionType)[keyof typeof ActionType];

export interface CreateAction {
export interface CreateAction<L1 extends TItemLocation, L2 extends TItemLocation> {
type: 'CREATE',
payload: TItem,
oldItem?: TItem,
payload: TItem<L1>,
oldItem?: TItem<L2>,
index?: number,
oldIndex?: number,
}

export interface UpdateAction {
export interface UpdateAction<L1 extends TItemLocation, L2 extends TItemLocation> {
type: 'UPDATE',
payload: TItem,
oldItem?: TItem,
payload: TItem<L1>,
oldItem?: TItem<L2>,
}

export interface RemoveAction {
export interface RemoveAction<L1 extends TItemLocation, L2 extends TItemLocation> {
type: 'REMOVE',
payload: TItem,
oldItem?: TItem,
payload: TItem<L1>,
oldItem?: TItem<L2>,
index?: number,
oldIndex?: number,
}

export interface ReorderAction {
export interface ReorderAction<L1 extends TItemLocation, L2 extends TItemLocation> {
type: 'REORDER',
payload: TItem,
oldItem?: TItem,
order: Ordering,
oldOrder?: Ordering,
payload: TItem<L1>,
oldItem?: TItem<L2>,
order: Ordering<L1>,
oldOrder?: Ordering<L2>,
}

export interface MoveAction {
export interface MoveAction<L1 extends TItemLocation, L2 extends TItemLocation> {
type: 'MOVE',
payload: TItem,
oldItem: TItem,
payload: TItem<L1>,
oldItem?: TItem<L2>,
index?: number,
oldIndex?: number,
}

export type Action = CreateAction|UpdateAction|RemoveAction|ReorderAction|MoveAction
export type Action<L1 extends TItemLocation, L2 extends TItemLocation> = CreateAction<L1, L2>|UpdateAction<L1, L2>|RemoveAction<L1, L2>|ReorderAction<L1,L2>|MoveAction<L1,L2>

export default class Diff {
private readonly actions: {
[ActionType.CREATE]: CreateAction[],
[ActionType.UPDATE]: UpdateAction[],
[ActionType.MOVE]: MoveAction[],
[ActionType.REMOVE]: RemoveAction[],
[ActionType.REORDER]: ReorderAction[]
}
export type LocationOfAction<A> = A extends Action<infer L, TItemLocation> ? L : never
export type OldLocationOfAction<A> = A extends Action<TItemLocation, infer L> ? L : never

export type MapLocation<A extends Action<TItemLocation, TItemLocation>, NewLocation extends TItemLocation> =
// eslint-disable-next-line no-unused-vars,@typescript-eslint/no-unused-vars
A extends CreateAction<infer O, infer P> ?
CreateAction<NewLocation, O>
// eslint-disable-next-line no-unused-vars,@typescript-eslint/no-unused-vars
: A extends UpdateAction<infer O, infer P> ?
UpdateAction<NewLocation, O>
// eslint-disable-next-line no-unused-vars,@typescript-eslint/no-unused-vars
: A extends MoveAction<infer O, infer P> ?
MoveAction<NewLocation, O>
// eslint-disable-next-line no-unused-vars,@typescript-eslint/no-unused-vars
: A extends RemoveAction<infer O, infer P> ?
RemoveAction<NewLocation, O>
// eslint-disable-next-line no-unused-vars,@typescript-eslint/no-unused-vars
: A extends ReorderAction<infer O, infer P> ?
ReorderAction<NewLocation, O>
: never

export default class Diff<L1 extends TItemLocation, L2 extends TItemLocation, A extends Action<L1, L2>> {
private readonly actions: A[]

constructor() {
this.actions = {
[ActionType.CREATE]: [],
[ActionType.UPDATE]: [],
[ActionType.MOVE]: [],
[ActionType.REMOVE]: [],
[ActionType.REORDER]: []
}
this.actions = []
}

clone(filter: (Action)=>boolean = () => true) {
const newDiff = new Diff
this.getActions().forEach((action: Action) => {
clone(filter: (action:A)=>boolean = () => true): Diff<L1, L2, A> {
const newDiff : Diff<L1, L2, A> = new Diff
this.getActions().forEach((action: A) => {
if (filter(action)) {
newDiff.commit(action)
}
Expand All @@ -84,59 +93,27 @@ export default class Diff {
return newDiff
}

commit(action: Action):void {
switch (action.type) {
case ActionType.CREATE:
this.actions[action.type].push({ ...action })
break
case ActionType.UPDATE:
this.actions[action.type].push({ ...action })
break
case ActionType.MOVE:
this.actions[action.type].push({ ...action })
break
case ActionType.REMOVE:
this.actions[action.type].push({ ...action })
break
case ActionType.REORDER:
this.actions[action.type].push({ ...action })
}
commit(action: A):void {
this.actions.push({ ...action })
}

retract(action: Action):void {
switch (action.type) {
case ActionType.CREATE:
this.actions[action.type].splice(this.actions[action.type].indexOf(action), 1)
break
case ActionType.UPDATE:
this.actions[action.type].splice(this.actions[action.type].indexOf(action), 1)
break
case ActionType.MOVE:
this.actions[action.type].splice(this.actions[action.type].indexOf(action), 1)
break
case ActionType.REMOVE:
this.actions[action.type].splice(this.actions[action.type].indexOf(action), 1)
break
case ActionType.REORDER:
this.actions[action.type].splice(this.actions[action.type].indexOf(action), 1)
break
}
retract(action: A):void {
this.actions.splice(this.actions.indexOf(action), 1)
}

getActions(type?: TActionType):Action[] {
if (type) {
return this.actions[type].slice()
}
getActions():A[] {
return [].concat(
this.actions[ActionType.UPDATE],
this.actions[ActionType.CREATE],
this.actions[ActionType.MOVE],
this.actions[ActionType.REMOVE],
this.actions[ActionType.REORDER],
this.actions
)
}

static findChain(mappingsSnapshot: MappingSnapshot, actions: Action[], itemTree: Folder, currentItem: TItem, targetAction: Action, chain: Action[] = []): boolean {
static findChain(
mappingsSnapshot: MappingSnapshot,
actions: Action<TItemLocation, TItemLocation>[], itemTree: Folder<TItemLocation>,
currentItem: TItem<TItemLocation>,
targetAction: Action<TItemLocation, TItemLocation>,
chain: Action<TItemLocation, TItemLocation>[] = []
): boolean {
const targetItemInTree = itemTree.findFolder(Mappings.mapId(mappingsSnapshot, targetAction.payload, itemTree.location))
if (
targetAction.payload.findItem(ItemType.FOLDER,
Expand All @@ -163,7 +140,7 @@ export default class Diff {
return false
}

static sortMoves(actions: Action[], tree: Folder) :Action[][] {
static sortMoves<L1 extends TItemLocation, L2 extends TItemLocation>(actions: MoveAction<L1, L2>[], tree: Folder<L1>) :MoveAction<L1, L2>[][] {
const bookmarks = actions.filter(a => a.payload.type === ItemType.BOOKMARK)
const folderMoves = actions.filter(a => a.payload.type === ItemType.FOLDER)
const DAG = folderMoves
Expand Down Expand Up @@ -199,17 +176,16 @@ export default class Diff {
* @param filter
* @param skipErroneousActions
*/
map(mappingsSnapshot:MappingSnapshot, targetLocation: TItemLocation, filter: (Action)=>boolean = () => true, skipErroneousActions = false): Diff {
const newDiff = new Diff
map<L3 extends TItemLocation>(mappingsSnapshot:MappingSnapshot, targetLocation: L3, filter: (action: A)=>boolean = () => true, skipErroneousActions = false): Diff<L3, L1, MapLocation<A, L3>> {
const newDiff : Diff<L3, L1, MapLocation<A, L3>> = new Diff

// Map payloads
this.getActions()
.map(a => a as Action)
.map(a => a as A)
.forEach(action => {
let newAction

if (!filter(action)) {
newDiff.commit(action)
return
}

Expand All @@ -224,23 +200,23 @@ export default class Diff {
const newId = action.payload.id
newAction = {
...action,
payload: action.payload.clone(false, targetLocation),
oldItem: action.oldItem.clone(false, action.payload.location)
payload: action.payload.cloneWithLocation(false, targetLocation),
oldItem: action.oldItem.cloneWithLocation(false, action.payload.location)
}
newAction.payload.id = oldId
newAction.oldItem.id = newId
} else {
newAction = {
...action,
payload: action.payload.clone(false, targetLocation),
payload: action.payload.cloneWithLocation(false, targetLocation),
oldItem: action.payload.clone(false)
}
newAction.payload.id = Mappings.mapId(mappingsSnapshot, action.payload, targetLocation)
}

if (oldItem && targetLocation !== ItemLocation.SERVER && action.type !== ActionType.MOVE && action.type !== ActionType.UPDATE) {
newAction.payload.parentId = action.oldItem.parentId
if (oldItem && targetLocation !== ItemLocation.SERVER && action.type !== ActionType.MOVE) {
newAction.oldItem.parentId = action.payload.parentId
newAction.payload.parentId = Mappings.mapParentId(mappingsSnapshot, action.oldItem, targetLocation)
} else {
newAction.oldItem.parentId = action.payload.parentId
newAction.payload.parentId = Mappings.mapParentId(mappingsSnapshot, action.payload, targetLocation)
Expand Down Expand Up @@ -270,7 +246,7 @@ export default class Diff {
}

toJSON() {
return this.getActions().map((action: Action) => {
return this.getActions().map((action: A) => {
return {
...action,
payload: action.payload.clone(false),
Expand All @@ -280,18 +256,50 @@ export default class Diff {
}

inspect(depth = 0):string {
return 'Diff\n' + this.getActions().map((action: Action) => {
return `\nAction: ${action.type}\nPayload: #${action.payload.id}[${action.payload.title}]${'url' in action.payload ? `(${action.payload.url})` : ''} ${'index' in action ? `Index: ${action.index}\n` : ''}${'order' in action ? `Order: ${JSON.stringify(action.order, null, '\t')}` : ''}`
return 'Diff\n' + this.getActions().map((action: A) => {
return `\nAction: ${action.type}\nPayload: #${action.payload.id}[${action.payload.title}]${'url' in action.payload ? `(${action.payload.url})` : ''} parentId: ${action.payload.parentId} ${'index' in action ? `Index: ${action.index}\n` : ''}${'order' in action ? `Order: ${JSON.stringify(action.order, null, '\t')}` : ''}`
}).join('\n')
}

static fromJSON(json) {
const diff = new Diff
json.forEach((action: Action): void => {
action.payload = hydrate(action.payload)
action.oldItem = action.oldItem && hydrate(action.oldItem)
static fromJSON<L1 extends TItemLocation, L2 extends TItemLocation, A2 extends Action<L1, L2>>(json) {
const diff: Diff<L1, L2, A2> = new Diff
json.forEach((action: A2): void => {
action.payload = hydrate<L1>(action.payload)
action.oldItem = action.oldItem && hydrate<L2>(action.oldItem)
diff.commit(action)
})
return diff
}
}

export interface PlanStage1<L1 extends TItemLocation, L2 extends TItemLocation> {
CREATE: Diff<L1, L2, CreateAction<L1, L2>>
UPDATE: Diff<L1, L2, UpdateAction<L1, L2>>
MOVE: Diff<L1, L2, MoveAction<L1, L2>>
REMOVE: Diff<L2, L1, RemoveAction<L2, L1>>
REORDER: Diff<L1, L2, ReorderAction<L1, L2>>
}

export interface PlanStage2<L1 extends TItemLocation, L2 extends TItemLocation, L3 extends TItemLocation> {
CREATE: Diff<L3, L1, CreateAction<L3, L1>>
UPDATE: Diff<L3, L1, UpdateAction<L3, L1>>
MOVE: Diff<L1, L2, MoveAction<L1, L2>>
REMOVE: Diff<L3, L2, RemoveAction<L3, L2>>
REORDER: Diff<L1, L2, ReorderAction<L1, L2>>
}

export interface PlanStage3<L1 extends TItemLocation, L2 extends TItemLocation, L3 extends TItemLocation> {
CREATE: Diff<L3, L1, CreateAction<L3, L1>>
UPDATE: Diff<L3, L1, UpdateAction<L3, L1>>
MOVE: Diff<L3, L1, MoveAction<L3, L1>>
REMOVE: Diff<L3, L2, RemoveAction<L3, L2>>
REORDER: Diff<L1, L2, ReorderAction<L1, L2>>
}

export interface PlanRevert<L1 extends TItemLocation, L2 extends TItemLocation> {
CREATE: Diff<L1, L2, CreateAction<L1, L2>>
UPDATE: Diff<L1, L2, UpdateAction<L1, L2>>
MOVE: Diff<L2, L1, MoveAction<L2, L1>>
REMOVE: Diff<L1, L2, RemoveAction<L1, L2>>
REORDER: Diff<L1, L2, ReorderAction<L1, L2>>
}
Loading
Loading