Skip to content

Commit d5db3bf

Browse files
marcelklehrmarcelklehr
committed
refactor(Folder#clone): Make Folder#clone use prototype inheritance
and use Folder#copy for all instances where it seems safer Signed-off-by: Marcel Klehr <[email protected]>
1 parent 7351d1b commit d5db3bf

File tree

11 files changed

+114
-42
lines changed

11 files changed

+114
-42
lines changed

‎src/lib/CachingTreeWrapper.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export default class CachingTreeWrapper implements OrderFolderResource<typeof It
2020

2121
async createBookmark(bookmark:Bookmark<typeof ItemLocation.LOCAL>): Promise<string|number> {
2222
const id = await this.innerTree.createBookmark(bookmark)
23-
const cacheId = await this.cacheTree.createBookmark(bookmark.clone(false))
23+
const cacheId = await this.cacheTree.createBookmark(bookmark.copy(false))
2424
const cacheBookmark = this.cacheTree.bookmarksCache.findBookmark(cacheId)
2525
cacheBookmark.id = id
2626
cacheBookmark.parentId = bookmark.parentId
@@ -30,7 +30,7 @@ export default class CachingTreeWrapper implements OrderFolderResource<typeof It
3030

3131
async updateBookmark(bookmark:Bookmark<typeof ItemLocation.LOCAL>):Promise<void> {
3232
await this.innerTree.updateBookmark(bookmark)
33-
await this.cacheTree.updateBookmark(bookmark.clone(false))
33+
await this.cacheTree.updateBookmark(bookmark.copy(false))
3434
}
3535

3636
async removeBookmark(bookmark:Bookmark<typeof ItemLocation.LOCAL>): Promise<void> {
@@ -40,7 +40,7 @@ export default class CachingTreeWrapper implements OrderFolderResource<typeof It
4040

4141
async createFolder(folder:Folder<typeof ItemLocation.LOCAL>): Promise<string|number> {
4242
const id = await this.innerTree.createFolder(folder)
43-
const cacheId = await this.cacheTree.createFolder(folder.clone(false))
43+
const cacheId = await this.cacheTree.createFolder(folder.copy(false))
4444
const cacheFolder = this.cacheTree.bookmarksCache.findFolder(cacheId)
4545
cacheFolder.id = id
4646
cacheFolder.parentId = folder.parentId
@@ -55,7 +55,7 @@ export default class CachingTreeWrapper implements OrderFolderResource<typeof It
5555

5656
async updateFolder(folder:Folder<typeof ItemLocation.LOCAL>): Promise<void> {
5757
await this.innerTree.updateFolder(folder)
58-
await this.cacheTree.updateFolder(folder.clone(false))
58+
await this.cacheTree.updateFolder(folder.copy(false))
5959
}
6060

6161
async removeFolder(folder:Folder<typeof ItemLocation.LOCAL>): Promise<void> {

‎src/lib/Diff.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,16 +200,16 @@ export default class Diff<L1 extends TItemLocation, L2 extends TItemLocation, A
200200
const newId = action.payload.id
201201
newAction = {
202202
...action,
203-
payload: action.payload.cloneWithLocation(false, targetLocation),
204-
oldItem: action.oldItem.cloneWithLocation(false, action.payload.location)
203+
payload: action.payload.copyWithLocation(false, targetLocation),
204+
oldItem: action.oldItem.copyWithLocation(false, action.payload.location)
205205
}
206206
newAction.payload.id = oldId
207207
newAction.oldItem.id = newId
208208
} else {
209209
newAction = {
210210
...action,
211-
payload: action.payload.cloneWithLocation(false, targetLocation),
212-
oldItem: action.payload.clone(false)
211+
payload: action.payload.copyWithLocation(false, targetLocation),
212+
oldItem: action.payload.copy(false)
213213
}
214214
newAction.payload.id = Mappings.mapId(mappingsSnapshot, action.payload, targetLocation)
215215
}
@@ -253,8 +253,8 @@ export default class Diff<L1 extends TItemLocation, L2 extends TItemLocation, A
253253
return this.getActions().map((action: A) => {
254254
return {
255255
...action,
256-
payload: action.payload.clone(false),
257-
oldItem: action.oldItem && action.oldItem.clone(false),
256+
payload: action.payload.copy(false),
257+
oldItem: action.oldItem && action.oldItem.copy(false),
258258
}
259259
})
260260
}

‎src/lib/Scanner.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
203203
this.result.REMOVE.retract(removeAction)
204204
} else {
205205
// We clone the item here, because we don't want to mutate all copies of this tree (item)
206-
const removedItemClone = removedItem.clone(true)
206+
const removedItemClone = removedItem.copy(true)
207207
const oldParentClone = removedItemClone.findItem(ItemType.FOLDER, oldItem.parentId) as Folder<L1>
208208
const oldItemClone = removedItemClone.findItem(oldItem.type, oldItem.id)
209209
oldIndex = oldParentClone.children.indexOf(oldItemClone)
@@ -235,7 +235,7 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
235235
this.result.CREATE.retract(createAction)
236236
} else {
237237
// We clone the item here, because we don't want to mutate all copies of this tree (item)
238-
const createdItemClone = createdItem.clone(true)
238+
const createdItemClone = createdItem.copy(true)
239239
const newParentClone = createdItemClone.findItem(ItemType.FOLDER, newItem.parentId) as Folder<L2>
240240
const newClonedItem = createdItemClone.findItem(newItem.type, newItem.id)
241241
index = newParentClone.children.indexOf(newClonedItem)

‎src/lib/Tree.ts‎

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,45 @@ export class Bookmark<L extends TItemLocation> {
8888
}
8989

9090
clone(withHash?: boolean):Bookmark<L> {
91-
return new Bookmark(this)
91+
return Object.create(this)
9292
}
9393

9494
cloneWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2): Bookmark<L2> {
95+
const newBookmark = Object.create(this)
96+
newBookmark.location = location
97+
return newBookmark
98+
}
99+
100+
copy(withHash?: boolean):Bookmark<L> {
101+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
102+
// @ts-ignore
103+
return new Bookmark(this.toJSON())
104+
}
105+
106+
copyWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2): Bookmark<L2> {
107+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
108+
// @ts-ignore
95109
return new Bookmark({
96-
...this,
110+
...this.toJSON(),
97111
location,
98112
})
99113
}
100114

115+
toJSON() {
116+
// Flatten inherited properties for serialization
117+
const result = {}
118+
let obj = this
119+
while (obj) {
120+
Object.entries(obj).forEach(([key, value]) => {
121+
if (!(key in result)) {
122+
result[key] = value
123+
}
124+
})
125+
obj = Object.getPrototypeOf(obj)
126+
}
127+
return result
128+
}
129+
101130
createIndex():any {
102131
return { [this.id]: this }
103132
}
@@ -309,23 +338,64 @@ export class Folder<L extends TItemLocation> {
309338
return this.hashValue[String(preserveOrder)]
310339
}
311340

312-
clone(withHash?:boolean):Folder<L> {
341+
copy(withHash?:boolean):Folder<L> {
342+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
343+
// @ts-ignore
313344
return new Folder({
314-
...this,
345+
...this.toJSON(),
315346
...(!withHash && { hashValue: {} }),
316-
children: this.children.map(child => child.clone(withHash))
347+
children: this.children.map(child => child.copy(withHash))
317348
})
318349
}
319350

320-
cloneWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2):Folder<L2> {
351+
copyWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2):Folder<L2> {
352+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
353+
// @ts-ignore
321354
return new Folder({
322-
...this,
355+
...this.toJSON(),
323356
location,
324357
...(!withHash && { hashValue: {} }),
325-
children: this.children.map(child => child.cloneWithLocation(withHash, location))
358+
children: this.children.map(child => child.copyWithLocation(withHash, location))
326359
})
327360
}
328361

362+
clone(withHash?:boolean):Folder<L> {
363+
const newFolder = Object.create(this)
364+
newFolder.index = null
365+
if (!withHash) {
366+
newFolder.hashValue = {}
367+
}
368+
newFolder.children = this.children.map(child => child.clone(withHash))
369+
return newFolder
370+
}
371+
372+
cloneWithLocation<L2 extends TItemLocation>(withHash:boolean, location: L2):Folder<L2> {
373+
const newFolder = Object.create(this)
374+
if (!withHash) {
375+
newFolder.hashValue = {}
376+
}
377+
newFolder.index = null
378+
newFolder.location = location
379+
newFolder.children = this.children.map(child => child.cloneWithLocation(withHash, location))
380+
return newFolder
381+
}
382+
383+
toJSON(): Folder<L> {
384+
// Flatten inherited properties for serialization
385+
const result: Folder<L> = {} as any as Folder<L>
386+
let obj = this
387+
while (obj) {
388+
Object.entries(obj).forEach(([key, value]) => {
389+
if (key === 'index') return
390+
if (!(key in result)) {
391+
result[key] = value
392+
}
393+
})
394+
obj = Object.getPrototypeOf(obj)
395+
}
396+
return result
397+
}
398+
329399
count():number {
330400
if (!this.index) {
331401
this.createIndex()

‎src/lib/adapters/Caching.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
3535
}
3636

3737
async getBookmarksTree(): Promise<Folder<TItemLocation>> {
38-
return this.bookmarksCache.clone()
38+
return this.bookmarksCache.copy()
3939
}
4040

4141
acceptsBookmark(bm:Bookmark<TItemLocation>):boolean {
@@ -53,6 +53,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
5353

5454
async createBookmark(bm:Bookmark<TItemLocation>):Promise<string|number> {
5555
Logger.log('CREATE', bm)
56+
bm = bm.copy()
5657
bm.id = ++this.highestId
5758
const foundFolder = this.bookmarksCache.findFolder(bm.parentId)
5859
if (!foundFolder) {
@@ -207,7 +208,7 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
207208
throw new UnknownCreateTargetError()
208209
}
209210
// clone and adjust ids
210-
const imported = folder.clone()
211+
const imported = folder.copy()
211212
imported.id = id
212213
await imported.traverse(async(item, parentFolder) => {
213214
item.id = ++this.highestId

‎src/lib/adapters/NextcloudBookmarks.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
297297

298298
tree.children = await this._getChildren(tree.id, -1)
299299
this.tree = tree
300-
return tree.clone()
300+
return tree.copy()
301301
}
302302

303303
async getSparseBookmarksTree() :Promise<Folder<typeof ItemLocation.SERVER>> {
@@ -310,7 +310,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
310310
this.list = null
311311
tree.loaded = false
312312
tree.hashValue = { true: await this._getFolderHash(tree.id) }
313-
this.tree = tree.clone(true) // we clone (withHash), so we can mess with our own version
313+
this.tree = tree.copy(true) // we clone (withHash), so we can mess with our own version
314314
return tree
315315
}
316316

@@ -368,7 +368,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
368368
throw new Error('Could not find folder for loadFolderChildren')
369369
}
370370
if (folder.loaded) {
371-
return folder.clone(true).children
371+
return folder.copy(true).children
372372
}
373373
let children
374374
if (all) {
@@ -392,7 +392,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
392392
folder.children = children
393393
folder.loaded = true
394394
this.tree.createIndex()
395-
return folder.clone(true).children
395+
return folder.copy(true).children
396396
}
397397

398398
async createFolder(folder:Folder<typeof ItemLocation.SERVER>):Promise<string|number> {
@@ -489,7 +489,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
489489
})
490490
}
491491
const imported = recurseChildren(json.data, parentId, folder.title, folder.parentId)
492-
parentFolder.children = imported.clone(true).children
492+
parentFolder.children = imported.copy(true).children
493493
this.tree.createIndex()
494494
return imported
495495
}
@@ -646,7 +646,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
646646
const existingBookmark = await this.getExistingBookmark(bm.url)
647647
if (existingBookmark) {
648648
bm.id = existingBookmark.id + ';' + bm.parentId // We already use the new parentId here, to avoid moving it away from the old location
649-
const updatedBookmark = bm.clone()
649+
const updatedBookmark = bm.copy()
650650
updatedBookmark.title = existingBookmark.title
651651
await this.updateBookmark(updatedBookmark)
652652
} else {
@@ -676,7 +676,7 @@ export default class NextcloudBookmarksAdapter implements Adapter, BulkImportRes
676676
bm.id = json.item.id + ';' + bm.parentId
677677
}
678678
// add bookmark to cached list
679-
const upstreamMark = bm.clone()
679+
const upstreamMark = bm.copy()
680680
upstreamMark.id = bm.id.split(';')[0]
681681
this.list && this.list.push(upstreamMark)
682682
if (this.tree) {

‎src/lib/native/NativeTree.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export default class NativeTree extends CachingAdapter implements BulkImportReso
2323
const {value: highestId} = await Storage.get({key: `bookmarks[${this.accountId}].highestId`})
2424
if (tree) {
2525
const oldHash = this.bookmarksCache && await this.bookmarksCache.cloneWithLocation(false, this.location).hash(true)
26-
this.bookmarksCache = Folder.hydrate(JSON.parse(tree)).clone(false)
26+
this.bookmarksCache = Folder.hydrate(JSON.parse(tree)).copy(false)
2727
const newHash = await this.bookmarksCache.hash(true)
2828
this.highestId = parseInt(highestId)
2929
return oldHash && oldHash !== newHash

‎src/lib/strategies/Default.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,7 @@ export default class SyncProcess {
669669
const originalCreation = sourceCreations.find(creation => creation.payload.findItem(ItemType.FOLDER, action.payload.parentId))
670670

671671
// Remove subitems that have been (re)moved already by other actions
672-
const newPayload = action.payload.clone()
672+
const newPayload = action.payload.copy()
673673
if (newPayload.type === ItemType.FOLDER) {
674674
newPayload.traverse((item, folder) => {
675675
const removed = sourceRemovals.find(a => Mappings.mappable(mappingsSnapshot, item, a.payload))
@@ -701,8 +701,8 @@ export default class SyncProcess {
701701

702702
concurrentHierarchyReversals.forEach(a => {
703703
// moved sourcely but moved in reverse hierarchical order on target
704-
const payload = a.oldItem.cloneWithLocation(false, action.payload.location)
705-
const oldItem = a.payload.cloneWithLocation(false, action.oldItem.location)
704+
const payload = a.oldItem.copyWithLocation(false, action.payload.location)
705+
const oldItem = a.payload.copyWithLocation(false, action.oldItem.location)
706706
oldItem.id = Mappings.mapId(mappingsSnapshot, a.payload, action.oldItem.location)
707707
oldItem.parentId = Mappings.mapParentId(mappingsSnapshot, a.payload, action.oldItem.location)
708708

@@ -876,6 +876,7 @@ export default class SyncProcess {
876876
return
877877
}
878878

879+
action.payload = action.payload.copy()
879880
action.payload.id = id
880881

881882
if (action.oldItem) {
@@ -897,7 +898,7 @@ export default class SyncProcess {
897898
Logger.log('Attempting full bulk import')
898899
try {
899900
// Try bulk import with sub folders
900-
const imported = await resource.bulkImportFolder(id, action.oldItem.cloneWithLocation(false, action.payload.location)) as Folder<typeof targetLocation>
901+
const imported = await resource.bulkImportFolder(id, action.oldItem.copyWithLocation(false, action.payload.location)) as Folder<typeof targetLocation>
901902
const newMappings = []
902903
const subScanner = new Scanner(
903904
action.oldItem,
@@ -953,7 +954,7 @@ export default class SyncProcess {
953954
} else {
954955
try {
955956
// Try bulk import without sub folders
956-
const tempItem = action.oldItem.cloneWithLocation(false, action.payload.location)
957+
const tempItem = action.oldItem.copyWithLocation(false, action.payload.location)
957958
const bookmarks = tempItem.children.filter(child => child instanceof Bookmark)
958959
while (bookmarks.length > 0) {
959960
Logger.log('Attempting chunked bulk import')

‎src/lib/strategies/Merge.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ export default class MergeSyncProcess extends DefaultSyncProcess {
133133
if (targetLocation === ItemLocation.SERVER) {
134134
concurrentHierarchyReversals.forEach(a => {
135135
// moved locally but moved in reverse hierarchical order on server
136-
const payload = a.oldItem.cloneWithLocation(false, action.payload.location) // we don't map here as we want this to look like a local action
137-
const oldItem = a.payload.cloneWithLocation(false, action.oldItem.location)
136+
const payload = a.oldItem.copyWithLocation(false, action.payload.location) // we don't map here as we want this to look like a local action
137+
const oldItem = a.payload.copyWithLocation(false, action.oldItem.location)
138138
oldItem.id = Mappings.mapId(mappingsSnapshot, oldItem, action.payload.location)
139139
oldItem.parentId = Mappings.mapParentId(mappingsSnapshot, oldItem, action.payload.location)
140140

‎src/lib/strategies/Unidirectional.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export default class UnidirectionalSyncProcess extends DefaultStrategy {
246246
}
247247

248248
private async translateCompleteItem<L1 extends TItemLocation, L2 extends TItemLocation>(item: TItem<L1>, mappingsSnapshot: MappingSnapshot, fakeLocation: L2) {
249-
const newItem = item.cloneWithLocation(false, fakeLocation)
249+
const newItem = item.copyWithLocation(false, fakeLocation)
250250
newItem.id = Mappings.mapId(mappingsSnapshot, item, fakeLocation)
251251
newItem.parentId = Mappings.mapParentId(mappingsSnapshot, item, fakeLocation)
252252
if (newItem instanceof Folder) {

0 commit comments

Comments
 (0)