-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Improve performance of Image and Gallery block processors to avoid long delay when saving a post #22896
Improve performance of Image and Gallery block processors to avoid long delay when saving a post #22896
Changes from all commits
14d9ddf
231f595
af3c5ad
8ba73eb
e3e8eb9
a22f0a5
f73df72
cbbd09d
32f122b
f9eadad
ccffe5b
cd1b26e
d3e4077
527d98d
5612643
3a22f64
a9c30fb
5733b49
d60c1d1
7694cb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -721,26 +721,30 @@ class PostCoordinator: NSObject { | |
} | ||
|
||
// Ensure that all synced media references are up to date | ||
post.media.forEach { media in | ||
if media.remoteStatus == .sync { | ||
self.updateReferences(to: media, in: post) | ||
} | ||
} | ||
let syncedMedia = post.media.filter { $0.remoteStatus == .sync } | ||
updateMediaBlocksBeforeSave(in: post, with: syncedMedia) | ||
|
||
let uuid = observeMedia(for: post, completion: completion) | ||
trackObserver(receipt: uuid, for: post) | ||
|
||
return | ||
} else { | ||
// Ensure that all media references are up to date | ||
post.media.forEach { media in | ||
self.updateReferences(to: media, in: post) | ||
} | ||
updateMediaBlocksBeforeSave(in: post, with: post.media) | ||
} | ||
|
||
completion(.success(post)) | ||
} | ||
|
||
func updateMediaBlocksBeforeSave(in post: AbstractPost, with media: Set<Media>) { | ||
guard let postContent = post.content else { | ||
return | ||
} | ||
let contentParser = GutenbergContentParser(for: postContent) | ||
media.forEach { self.updateReferences(to: $0, in: contentParser.blocks, post: post) } | ||
post.content = contentParser.html() | ||
} | ||
|
||
// - warning: deprecated (kahu-offline-mode) | ||
func cancelAnyPendingSaveOf(post: AbstractPost) { | ||
removeObserver(for: post) | ||
|
@@ -879,7 +883,7 @@ class PostCoordinator: NSObject { | |
switch state { | ||
case .ended: | ||
let successHandler = { | ||
self.updateReferences(to: media, in: post) | ||
self.updateMediaBlocksBeforeSave(in: post, with: [media]) | ||
if self.isSyncPublishingEnabled { | ||
if post.media.allSatisfy({ $0.remoteStatus == .sync }) { | ||
self.removeObserver(for: post) | ||
|
@@ -915,7 +919,7 @@ class PostCoordinator: NSObject { | |
}, forMediaFor: post) | ||
} | ||
|
||
private func updateReferences(to media: Media, in post: AbstractPost) { | ||
private func updateReferences(to media: Media, in contentBlocks: [GutenbergParsedBlock], post: AbstractPost) { | ||
guard var postContent = post.content, | ||
let mediaID = media.mediaID?.intValue, | ||
let remoteURLStr = media.remoteURL else { | ||
|
@@ -935,19 +939,21 @@ class PostCoordinator: NSObject { | |
if media.remoteStatus == .failed { | ||
return | ||
} | ||
var gutenbergProcessors = [Processor]() | ||
var aztecProcessors = [Processor]() | ||
|
||
var gutenbergBlockProcessors: [GutenbergProcessor] = [] | ||
var gutenbergProcessors: [Processor] = [] | ||
Comment on lines
+943
to
+944
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea is to update gradually the processors to adopt the new |
||
var aztecProcessors: [Processor] = [] | ||
|
||
// File block can upload any kind of media. | ||
let gutenbergFileProcessor = GutenbergFileUploadProcessor(mediaUploadID: gutenbergMediaUploadID, serverMediaID: mediaID, remoteURLString: remoteURLStr) | ||
gutenbergProcessors.append(gutenbergFileProcessor) | ||
gutenbergBlockProcessors.append(gutenbergFileProcessor) | ||
|
||
if media.mediaType == .image { | ||
let gutenbergImgPostUploadProcessor = GutenbergImgUploadProcessor(mediaUploadID: gutenbergMediaUploadID, serverMediaID: mediaID, remoteURLString: imageURL) | ||
gutenbergProcessors.append(gutenbergImgPostUploadProcessor) | ||
gutenbergBlockProcessors.append(gutenbergImgPostUploadProcessor) | ||
|
||
let gutenbergGalleryPostUploadProcessor = GutenbergGalleryUploadProcessor(mediaUploadID: gutenbergMediaUploadID, serverMediaID: mediaID, remoteURLString: imageURL, mediaLink: mediaLink) | ||
gutenbergProcessors.append(gutenbergGalleryPostUploadProcessor) | ||
gutenbergBlockProcessors.append(gutenbergGalleryPostUploadProcessor) | ||
|
||
let imgPostUploadProcessor = ImgUploadProcessor(mediaUploadID: mediaUploadID, remoteURLString: remoteURLStr, width: media.width?.intValue, height: media.height?.intValue) | ||
aztecProcessors.append(imgPostUploadProcessor) | ||
|
@@ -980,6 +986,7 @@ class PostCoordinator: NSObject { | |
} | ||
|
||
// Gutenberg processors need to run first because they are more specific/and target only content inside specific blocks | ||
gutenbergBlockProcessors.forEach { $0.process(contentBlocks) } | ||
postContent = gutenbergProcessors.reduce(postContent) { (content, processor) -> String in | ||
return processor.process(content) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import Foundation | ||
import Aztec | ||
import SwiftSoup | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aztec was imported in this file to use |
||
|
||
class GutenbergGalleryUploadProcessor: Processor { | ||
class GutenbergGalleryUploadProcessor: GutenbergProcessor { | ||
|
||
let mediaUploadID: Int32 | ||
let remoteURLString: String | ||
|
@@ -28,76 +28,70 @@ class GutenbergGalleryUploadProcessor: Processor { | |
static let dataLink = "data-link" | ||
} | ||
|
||
lazy var imgPostMediaUploadProcessor = HTMLProcessor(for: ImageKeys.name, replacer: { (img) in | ||
guard let imgClassAttributeValue = img.attributes[ImageKeys.classAttributes]?.value, | ||
case let .string(imgClass) = imgClassAttributeValue else { | ||
return nil | ||
func processImgPostMediaUpload(_ element: Element) { | ||
guard let imgTags = try? element.select(ImageKeys.name) else { | ||
return | ||
} | ||
imgTags.forEach {imgTag in | ||
guard let imgClass = try? imgTag.attr(ImageKeys.classAttributes) else { | ||
return | ||
} | ||
|
||
let classAttributes = imgClass.components(separatedBy: " ") | ||
let classAttributes = imgClass.components(separatedBy: " ") | ||
|
||
guard let imageIDAttribute = classAttributes.filter({ (value) -> Bool in | ||
value.hasPrefix(GutenbergImgUploadProcessor.imgClassIDPrefixAttribute) | ||
}).first else { | ||
return nil | ||
} | ||
guard let imageIDAttribute = classAttributes.filter({ (value) -> Bool in | ||
value.hasPrefix(GutenbergImgUploadProcessor.imgClassIDPrefixAttribute) | ||
}).first else { | ||
return | ||
} | ||
|
||
let imageIDString = String(imageIDAttribute.dropFirst(ImageKeys.classIDPrefix.count)) | ||
let imgUploadID = Int32(imageIDString) | ||
let imageIDString = String(imageIDAttribute.dropFirst(ImageKeys.classIDPrefix.count)) | ||
let imgUploadID = Int32(imageIDString) | ||
|
||
guard imgUploadID == self.mediaUploadID else { | ||
return nil | ||
} | ||
guard imgUploadID == self.mediaUploadID else { | ||
return | ||
} | ||
|
||
let newImgClassAttributes = imgClass.replacingOccurrences(of: imageIDAttribute, with: ImageKeys.classIDPrefix + String(self.serverMediaID)) | ||
let newImgClassAttributes = imgClass.replacingOccurrences(of: imageIDAttribute, with: ImageKeys.classIDPrefix + String(self.serverMediaID)) | ||
|
||
var attributes = img.attributes | ||
attributes.set(.string(self.remoteURLString), forKey: "src") | ||
attributes.set(.string(newImgClassAttributes), forKey: "class") | ||
attributes.set(.string("\(self.serverMediaID)"), forKey: ImageKeys.dataID) | ||
attributes.set(.string(self.remoteURLString), forKey: ImageKeys.dataFullURL) | ||
if attributes.contains(where: { $0.key == ImageKeys.dataLink } ) { | ||
attributes.set(.string(self.mediaLink), forKey: ImageKeys.dataLink) | ||
} | ||
_ = try? imgTag.attr("src", self.remoteURLString) | ||
_ = try? imgTag.attr("class", newImgClassAttributes) | ||
_ = try? imgTag.attr(ImageKeys.dataID, String(self.serverMediaID)) | ||
_ = try? imgTag.attr(ImageKeys.dataFullURL, self.remoteURLString) | ||
|
||
var html = "<\(ImageKeys.name) " | ||
let attributeSerializer = ShortcodeAttributeSerializer() | ||
html += attributeSerializer.serialize(attributes) | ||
html += " />" | ||
return html | ||
}) | ||
if let _ = try? imgTag.attr(ImageKeys.dataLink) { | ||
_ = try? imgTag.attr(ImageKeys.dataLink, self.mediaLink) | ||
} | ||
} | ||
} | ||
|
||
private struct LinkKeys { | ||
static let name = "a" | ||
} | ||
|
||
lazy var linkPostMediaUploadProcessor = HTMLProcessor(for: LinkKeys.name, replacer: { (link) in | ||
|
||
guard let linkOriginalContent = link.content else { | ||
return nil | ||
func processLinkPostMediaUpload(_ block: GutenbergParsedBlock) { | ||
guard let aTags = try? block.elements.select(LinkKeys.name) else { | ||
return | ||
} | ||
aTags.forEach { aTag in | ||
guard let linkOriginalContent = try? aTag.html() else { | ||
return | ||
} | ||
|
||
let linkUpdatedContent = self.imgPostMediaUploadProcessor.process(linkOriginalContent) | ||
processImgPostMediaUpload(aTag) | ||
let linkUpdatedContent = try? aTag.html() | ||
|
||
guard linkUpdatedContent != linkOriginalContent else { | ||
return nil | ||
} | ||
guard linkUpdatedContent != linkOriginalContent else { | ||
return | ||
} | ||
|
||
var attributes = link.attributes | ||
if let linkToURL = self.linkToURL { | ||
attributes.set(.string(linkToURL), forKey: "href") | ||
} else { | ||
attributes.set(.string(self.remoteURLString), forKey: "href") | ||
if let linkToURL = self.linkToURL { | ||
_ = try? aTag.attr("href", linkToURL) | ||
} else { | ||
_ = try? aTag.attr("href", self.remoteURLString) | ||
} | ||
} | ||
|
||
var html = "<\(LinkKeys.name) " | ||
let attributeSerializer = ShortcodeAttributeSerializer() | ||
html += attributeSerializer.serialize(attributes) | ||
html += " >" | ||
html += linkUpdatedContent | ||
html += "</\(LinkKeys.name)>" | ||
return html | ||
}) | ||
} | ||
|
||
private struct GalleryBlockKeys { | ||
static let name = "wp:gallery" | ||
|
@@ -117,42 +111,35 @@ class GutenbergGalleryUploadProcessor: Processor { | |
return ids | ||
} | ||
|
||
lazy var galleryBlockProcessor = GutenbergBlockProcessor(for: GalleryBlockKeys.name, replacer: { block in | ||
guard let idsAny = block.attributes[GalleryBlockKeys.ids] as? [Any] else { | ||
return nil | ||
} | ||
var ids = self.convertToIntArray(idsAny) | ||
guard ids.contains(self.mediaUploadID) else { | ||
return nil | ||
} | ||
var updatedBlock = "<!-- \(GalleryBlockKeys.name) " | ||
var attributes = block.attributes | ||
if let index = ids.firstIndex(of: self.mediaUploadID ) { | ||
ids[index] = Int32(self.serverMediaID) | ||
} | ||
attributes[GalleryBlockKeys.ids] = ids | ||
|
||
if let jsonData = try? JSONSerialization.data(withJSONObject: attributes, options: .sortedKeys), | ||
let jsonString = String(data: jsonData, encoding: .utf8) { | ||
updatedBlock += jsonString | ||
} | ||
updatedBlock += " -->" | ||
if let linkTo = block.attributes[GalleryBlockKeys.linkTo] as? String, linkTo != "none" { | ||
if linkTo == "file" { | ||
self.linkToURL = self.remoteURLString | ||
} else if linkTo == "post" { | ||
self.linkToURL = self.mediaLink | ||
func processGalleryBlocks(_ blocks: [GutenbergParsedBlock]) { | ||
let galleryBlocks = blocks.filter { $0.name == GalleryBlockKeys.name } | ||
galleryBlocks.forEach { block in | ||
guard let idsAny = block.attributes[GalleryBlockKeys.ids] as? [Any] else { | ||
return | ||
} | ||
var ids = self.convertToIntArray(idsAny) | ||
guard ids.contains(self.mediaUploadID) else { | ||
return | ||
} | ||
if let index = ids.firstIndex(of: self.mediaUploadID ) { | ||
ids[index] = Int32(self.serverMediaID) | ||
} | ||
block.attributes[GalleryBlockKeys.ids] = ids | ||
|
||
if let linkTo = block.attributes[GalleryBlockKeys.linkTo] as? String, linkTo != "none" { | ||
if linkTo == "file" { | ||
self.linkToURL = self.remoteURLString | ||
} else if linkTo == "post" { | ||
self.linkToURL = self.mediaLink | ||
} | ||
processLinkPostMediaUpload(block) | ||
} else { | ||
block.elements.forEach { processImgPostMediaUpload($0) } | ||
} | ||
updatedBlock += self.linkPostMediaUploadProcessor.process(block.content) | ||
} else { | ||
updatedBlock += self.imgPostMediaUploadProcessor.process(block.content) | ||
} | ||
updatedBlock += "<!-- /\(GalleryBlockKeys.name) -->" | ||
return updatedBlock | ||
}) | ||
} | ||
|
||
func process(_ text: String) -> String { | ||
let result = galleryBlockProcessor.process(text) | ||
return result | ||
func process(_ blocks: [GutenbergParsedBlock]) { | ||
processGalleryBlocks(blocks) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please be cautious about making changes to
PostCoordinator
because the large potion of it is or will be rewritten.prepareToSave
is unchanged, so it's a safe change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kean for the call out 🙇 ! This PR mainly focuses on the logic related to processing blocks before saving a post, which requires touching up some bits of the
PostCoordinator
. I'm planning to modify only necessary here, so hopefully there won't be much conflict with ongoing changes. In any case, I'll be happy to contribute to the new version by applying the same updates.