Skip to content

Re-implement building page tree hierarchy #22345

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.

Sign up for GitHub

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

Merged
mokagio merged 10 commits into from
Jan 9, 2024
Merged

Re-implement building page tree hierarchy #22345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bd50f67
Add a couple of unit tests to test PageTree
crazytonyli Jan 8, 2024
3eb5573
Fix #22283: child pages are not moved under parent pages
crazytonyli Jan 8, 2024
c705496
Add a release note
crazytonyli Jan 8, 2024
eac5db6
Merge remote trunk into fix-issue-22283-page-list-hierachy
mokagio Jan 8, 2024
3839c2f
Fix typo in `HierarchyList` name
mokagio Jan 8, 2024
7a834f0
Simpilify building page tree hierarchy
crazytonyli Jan 8, 2024
d087d78
Revert to more strict comparision
crazytonyli Jan 8, 2024
b118514
Update view controller to use the new function
crazytonyli Jan 8, 2024
49fd7b9
Remove no longer required try keywords
crazytonyli Jan 8, 2024
55fbfdb
Update PR number in a release note
crazytonyli Jan 8, 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
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [*] Fix an issue with BlogDashboardPersonalizationService being used on the background thread [#22335]
* [***] Block Editor: Avoid keyboard dismiss when interacting with text blocks [https://github.com/WordPress/gutenberg/pull/57070]
* [**] Block Editor: Auto-scroll upon block insertion [https://github.com/WordPress/gutenberg/pull/57273]
* [**] Fix an issue in Pages List where the pages are not displayed in a hierarchical order [#22345]

23.9
-----
Expand Down
230 changes: 33 additions & 197 deletions WordPress/Classes/Utility/PageTree.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,18 @@ final class PageTree {
var postID: NSNumber?
var parentID: NSNumber?
}
let pageID: TaggedManagedObjectID<Page>
let pageData: PageData
let page: Page
var children = [TreeNode]()
var parentNode: TreeNode?

init(page: Page, children: [TreeNode] = [], parentNode: TreeNode? = nil) {
self.pageID = TaggedManagedObjectID(page)
self.pageData = PageData(postID: page.postID, parentID: page.parentID)
self.children = children
self.parentNode = parentNode
init(page: Page) {
self.page = page
}

// The `PageTree` type is used to loaded
// Some page There are pages They are pages that doesn't belong to the root level, but their parent pages haven't been loaded yet.
var isOrphan: Bool {
(pageData.parentID?.int64Value ?? 0) > 0 && parentNode == nil
}

func dfsList(in context: NSManagedObjectContext) throws -> [Page] {
func dfsList() -> [Page] {
var pages = [Page]()
_ = try depthFirstSearch { level, node in
let page = try context.existingObject(with: node.pageID)
_ = depthFirstSearch { level, node in
let page = node.page
page.hierarchyIndex = level
page.hasVisibleParent = node.parentNode != nil
pages.append(page)
Expand All @@ -42,211 +32,57 @@ final class PageTree {
/// a boolean value indicate whether the search should be stopped.
/// - Returns: `true` if search has been stopped by the closure.
@discardableResult
func depthFirstSearch(using closure: (Int, TreeNode) throws -> Bool) rethrows -> Bool {
try depthFirstSearch(level: 0, using: closure)
func depthFirstSearch(using closure: (Int, TreeNode) -> Bool) -> Bool {
depthFirstSearch(level: 0, using: closure)
}

private func depthFirstSearch(level: Int, using closure: (Int, TreeNode) throws -> Bool) rethrows -> Bool {
let shouldStop = try closure(level, self)
private func depthFirstSearch(level: Int, using closure: (Int, TreeNode) -> Bool) -> Bool {
let shouldStop = closure(level, self)
if shouldStop {
return true
}

for child in children {
let shouldStop = try child.depthFirstSearch(level: level + 1, using: closure)
let shouldStop = child.depthFirstSearch(level: level + 1, using: closure)
if shouldStop {
return true
}
}

return false
}

/// Perform breadth-first search starting with the current (`self`) node.
///
/// - Parameter closure: A closure that takes a node as argument and returns a boolean value indicate whether
/// the search should be stopped.
/// - Returns: `true` if search has been stopped by the closure.
func breadthFirstSearch(using closure: (TreeNode) -> Bool) {
var queue = [TreeNode]()
queue.append(self)
while let current = queue.popLast() {
let shouldStop = closure(current)
if shouldStop {
break
}

queue.append(contentsOf: current.children)
}
}

func add(_ newNodes: [TreeNode], parentID: NSNumber) -> Bool {
assert(parentID != 0)

return depthFirstSearch { _, node in
if node.pageData.postID == parentID {
node.children.append(contentsOf: newNodes)
newNodes.forEach { $0.parentNode = node }
return true
}
return false
}
}
}

// The top level (or root level) pages, or nodes.
// They can be two types node:
// - child nodes. They are top level pages.
// - orphan nodes. They are pages that doesn't belong to the root level, but their parent pages haven't been loaded yet.
private var nodes = [TreeNode]()

// `orphanNodes` contains indexes of orphan nodes in the `nodes` array (the value part in the dictionary), which are
// grouped using their parent id (the key part in the dictionary).
// IMPORTANT: Make sure `orphanNodes` is up-to-date after the `nodes` array is modified.
private var orphanNodes = [NSNumber: [Int]]()

/// Add *new pages* to the page tree.
///
/// This function assumes none of array elements already exists in the current page tree.
func add(_ newPages: [Page]) {
let newNodes = newPages.map { TreeNode(page: $0) }
relocateOrphans(to: newNodes)

// First try to constrcuture a smaller subtree from the given pages, then move the new subtree to the existing
// page tree (`self`).
// The number of pages in a subtree can be changed if we want to futher tweak the performance.
let batch = 100
for index in stride(from: 0, to: newNodes.count, by: batch) {
let tree = PageTree()
tree.add(Array(newNodes[index..<min(index + batch, newNodes.count)]))
merge(subtree: tree)
}
}

/// Find the existing orphan nodes' parents in the given new nodes list argument and move them under their parent
/// node if found.
private func relocateOrphans(to newNodes: [TreeNode]) {
let relocated = orphanNodes.reduce(into: IndexSet()) { result, element in
let parentID = element.key
let indexes = element.value

let toBeRelocated = indexes.map { nodes[$0] }
let moved = newNodes.contains {
$0.add(toBeRelocated, parentID: parentID)
}
if moved {
result.formUnion(IndexSet(indexes))
}
}

if !relocated.isEmpty {
nodes.remove(atOffsets: relocated)
orphanNodes = nodes.enumerated().reduce(into: [:]) { indexes, node in
if node.element.isOrphan {
let parentID = node.element.pageData.parentID ?? 0
indexes[parentID, default: []].append(node.offset)
}
}
}
}

private func add(_ newNodes: [TreeNode]) {
newNodes.forEach { newNode in
let parentID = newNode.pageData.parentID ?? 0
static func hierarchyList(of pages: [Page]) -> [Page] {
// An array of `TreeNode` instances that are one-to-one map of the `pages` list.
var nodes: [TreeNode] = []
// A map of parent page (the dictionary key) to its children (the dictionary value).
var children: [NSNumber: [TreeNode]] = [:]
var allPostIDs: Set<NSNumber> = []

// If the new node is at the root level, then simply add it as a child.
if parentID == 0 {
nodes.append(newNode)
return
}

// The new node is not at the root level, find its parent in the root level nodes.
for child in nodes {
if child.add([newNode], parentID: parentID) {
break
}
}

// Still not find their parent, add it to the root level nodes.
if newNode.parentNode == nil {
nodes.append(newNode)
orphanNodes[parentID, default: []].append(nodes.count - 1)
}
for page in pages {
let node = TreeNode(page: page)
nodes.append(node)
allPostIDs.insert(page.postID ?? 0)
children[page.parentID ?? 0, default: []].append(node)
}
}

/// Move all the nodes in the given argument to the current page tree.
private func merge(subtree: PageTree) {
var parentIDs = subtree.nodes.reduce(into: Set()) { $0.insert($1.pageData.parentID ?? 0) }
// No need to look for root level
parentIDs.remove(0)
// Look up parent nodes upfront, to avoid repeated iteration for each node in `subtree`.
let parentNodes = findNodes(postIDs: parentIDs)

subtree.nodes.forEach { newNode in
let parentID = newNode.pageData.parentID ?? 0

// If the new node is at the root level, then simply add it as a child
if parentID == 0 {
nodes.append(newNode)
return
}

// The new node is not at the root level, find its parent in the root level nodes.
if let parentNode = parentNodes[parentID] {
parentNode.children.append(newNode)
newNode.parentNode = parentNode
} else {
// No parent found, add it to the root level nodes.
nodes.append(newNode)
orphanNodes[parentID, default: []].append(nodes.count - 1)
}
}
}

/// Find the node for the given page ids
private func findNodes(postIDs originalIDs: Set<NSNumber>) -> [NSNumber: TreeNode] {
guard !originalIDs.isEmpty else {
return [:]
// Move children nodes to through the given node and its descendants.
func addChildren(to node: TreeNode) {
node.children = children[node.page.postID ?? 0] ?? []
node.children.forEach(addChildren(to:))
}

var ids = originalIDs
var result = [NSNumber: TreeNode]()

// The new node is not at the root level, find its parent in the root level nodes.
for child in nodes {
if ids.isEmpty {
break
}

// Using BFS under the assumption that page tree in most sites is a shallow tree, where most pages are in top layers.
child.breadthFirstSearch { node in
let postID = node.pageData.postID ?? 0
let foundIndex = ids.firstIndex(of: postID)
if let foundIndex {
ids.remove(at: foundIndex)
result[postID] = node
}
return ids.isEmpty
}
// The top level nodes are pages whose parent id is 0 and pages whose parent page are not in the `pages` list.
let topLevelNodes = nodes.filter {
let parentID = $0.page.parentID ?? 0
return ($0.page.parentID ?? 0) == 0 || !allPostIDs.contains(parentID)
}

return result
}
topLevelNodes.forEach(addChildren(to:))

func hierarchyList(in context: NSManagedObjectContext) throws -> [Page] {
try nodes.reduce(into: []) {
try $0.append(contentsOf: $1.dfsList(in: context))
return topLevelNodes.reduce(into: []) {
$0.append(contentsOf: $1.dfsList())
}
}

static func hierarchyList(of pages: [Page]) throws -> [Page] {
guard let context = pages.first?.managedObjectContext else {
return []
}

let tree = PageTree()
tree.add(pages)
return try tree.hierarchyList(in: context)
}
}
22 changes: 16 additions & 6 deletions WordPress/Classes/ViewRelated/Pages/PageListViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro

do {
self.pages = try await buildPageTree(pageIDs: pageIDs)
.hierarchyList(in: coreDataStack.mainContext)
.map { pageID, hierarchyIndex in
let page = try coreDataStack.mainContext.existingObject(with: pageID)
page.hierarchyIndex = hierarchyIndex
return page
}
} catch {
DDLogError("Failed to reload published pages: \(error)")
}
Expand All @@ -263,7 +267,7 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro

/// Build page hierachy in background, which should not take long (less than 2 seconds for 6000+ pages).
@MainActor
func buildPageTree(pageIDs: [TaggedManagedObjectID<Page>]? = nil, request: NSFetchRequest<Page>? = nil) async throws -> PageTree {
func buildPageTree(pageIDs: [TaggedManagedObjectID<Page>]? = nil, request: NSFetchRequest<Page>? = nil) async throws -> [(pageID: TaggedManagedObjectID<Page>, hierarchyIndex: Int)] {
assert(pageIDs != nil || request != nil, "`pageIDs` and `request` can not both be nil")

let coreDataStack = ContextManager.shared
Expand All @@ -278,9 +282,9 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro

pages = pages.setHomePageFirst()

let tree = PageTree()
tree.add(pages)
return tree
// The `hierarchyIndex` is not a managed property, so it needs to be returend along with the page object id.
return PageTree.hierarchyList(of: pages)
.map { (TaggedManagedObjectID($0), $0.hierarchyIndex) }
}
}

Expand Down Expand Up @@ -462,7 +466,13 @@ final class PageListViewController: AbstractPostListViewController, UIViewContro
request.predicate = filter.predicate(for: blog, author: .everyone)
request.sortDescriptors = filter.sortDescriptors
do {
var pages = try await buildPageTree(request: request).hierarchyList(in: ContextManager.shared.mainContext)
let context = ContextManager.shared.mainContext
var pages = try await buildPageTree(request: request)
.map { pageID, hierarchyIndex in
let page = try context.existingObject(with: pageID)
page.hierarchyIndex = hierarchyIndex
return page
}
if let index = pages.firstIndex(of: page) {
pages = pages.remove(from: index)
}
Expand Down
Loading