Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
4 changes: 2 additions & 2 deletions Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def wordpress_ui
end

def wordpress_kit
pod 'WordPressKit', '~> 6.2.0-beta'
pod 'WordPressKit', '~> 7.0.0-beta'
# pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', branch: 'trunk'
# pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', tag: ''
# pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: ''
Expand Down Expand Up @@ -232,7 +232,7 @@ abstract_target 'Apps' do

pod 'Gridicons', '~> 1.1.0'

pod 'WordPressAuthenticator', '~> 5.1-beta'
pod 'WordPressAuthenticator', '~> 5.6-beta'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging the PR, probably we should check if there are any important changes between 5.1 to 5.6 in WordPressAuthenticator.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

The more interesting file to look at is Podfile.lock where WordPressAuthenticator results resolved as version 5.5.0.

The only differences between 5.5.0 and 5.6.0-beta.1 are of course all and only the changes bundled with 5.6.0-beta.1, and those are nothing more that the WordPressKit major version bump, which this PR verified.

I think this version requirement change is safe to merge. Even if an issue was introduced in going from ~> 5.1-beta to ~> 5.6-beta the culprit would not be in 5.6.0-beta.1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, was there a requirement in the code for something that came in WordPressAuthenticator 5.6.0?

Otherwise, we could have left the requirement as it was and trust in CocoaPods to resolve the correct WordPressAuthenticator version compatible with WordPressKit 7.0.0-beta.1.

Out of curiosity, I reverted this change on my machine and everything worked fine.

No big deal, though. We'll soon need to actually updated WordPressAuthenticator anyway: #20128

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only differences between 5.5.0 and 5.6.0-beta.1 are of course all and only the changes bundled with 5.6.0-beta.1, and those are nothing more that the WordPressKit major version bump, which this PR verified.

Ah, good point. I thought might be more changes but based on Podfile.lock the only one was introduced in 5.6.0-beta.1 🤦 .

By the way, was there a requirement in the code for something that came in WordPressAuthenticator 5.6.0?

Actually, there's no change required in this PR from WordPressAuthenticator. However, when I upgraded WordPressKit to version 7.0 (reference), I was getting an error when installing the WordPressAuthenticator pod related to the minimum version. Due to this, I had to create wordpress-mobile/WordPressAuthenticator-iOS#754 and bump the minimum version of WordPressKit dependency to 7.0.

However, I've reverted the changes as you mentioned and I'm not getting now the error 🤔.

# pod 'WordPressAuthenticator', git: 'https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git', branch: 'trunk'
# pod 'WordPressAuthenticator', git: 'https://github.com/wordpress-mobile/WordPressAuthenticator-iOS.git', commit: ''
# pod 'WordPressAuthenticator', path: '../WordPressAuthenticator-iOS'
Expand Down
18 changes: 9 additions & 9 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -503,15 +503,15 @@ PODS:
- WordPress-Aztec-iOS (1.19.8)
- WordPress-Editor-iOS (1.19.8):
- WordPress-Aztec-iOS (= 1.19.8)
- WordPressAuthenticator (5.5.0):
- WordPressAuthenticator (5.6.0-beta.1):
- GoogleSignIn (~> 6.0.1)
- Gridicons (~> 1.0)
- "NSURL+IDN (= 0.4)"
- SVProgressHUD (~> 2.2.5)
- WordPressKit (~> 6.0-beta)
- WordPressShared (~> 2.0-beta)
- WordPressKit (~> 7.0-beta)
- WordPressShared (~> 2.1-beta)
- WordPressUI (~> 1.7-beta)
- WordPressKit (6.2.0):
- WordPressKit (7.0.0-beta.1):
- Alamofire (~> 4.8.0)
- NSObject-SafeExpectations (~> 0.0.4)
- UIDeviceIdentifier (~> 2.0)
Expand Down Expand Up @@ -611,8 +611,8 @@ DEPENDENCIES:
- SVProgressHUD (= 2.2.5)
- SwiftLint (~> 0.50)
- WordPress-Editor-iOS (~> 1.19.8)
- WordPressAuthenticator (~> 5.1-beta)
- WordPressKit (~> 6.2.0-beta)
- WordPressAuthenticator (~> 5.6-beta)
- WordPressKit (~> 7.0.0-beta)
- WordPressShared (~> 2.0-beta)
- WordPressUI (~> 1.12.5)
- WPMediaPicker (~> 1.8.7)
Expand Down Expand Up @@ -880,8 +880,8 @@ SPEC CHECKSUMS:
UIDeviceIdentifier: e6a801d25f4f178de5bdf475ffe29050d0148176
WordPress-Aztec-iOS: 7d11d598f14c82c727c08b56bd35fbeb7dafb504
WordPress-Editor-iOS: 9eb9f12f21a5209cb837908d81ffe1e31cb27345
WordPressAuthenticator: 60611898815b218622f98d2c024d4eeae53dec1c
WordPressKit: 7fb291f67b4f2651f09ed3dd9c6010776d652165
WordPressAuthenticator: de6f43d5fb654dadbd0456434088c47b6f6e4e48
WordPressKit: d5bff8713aa7c0092ff6e2a58623e46a99fc897c
WordPressShared: 8e59bc8cec256f54a7c4cc6c94911adc2a9a65d2
WordPressUI: c5be816f6c7b3392224ac21de9e521e89fa108ac
WPMediaPicker: 0d45dfd7b3c5651c5236ffd48c1b0b2f60a2d5d2
Expand All @@ -896,6 +896,6 @@ SPEC CHECKSUMS:
ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba
ZIPFoundation: ae5b4b813d216d3bf0a148773267fff14bd51d37

PODFILE CHECKSUM: 6beaf4f4ed2d089e9fa8d281c83f74dc8f508fa1
PODFILE CHECKSUM: 32a512e8f68990bf175a93cb2bf2ebfa3c17b16b

COCOAPODS: 1.11.3
11 changes: 9 additions & 2 deletions WordPress/Classes/Categories/Media+WPMediaAsset.m
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,16 @@ - (WPMediaRequestID)videoAssetWithCompletionHandler:(WPMediaAssetBlock)completio
if (!url && self.videopressGUID.length > 0 ){
NSManagedObjectContext *mainContext = [[ContextManager sharedInstance] mainContext];
MediaService *mediaService = [[MediaService alloc] initWithManagedObjectContext:mainContext];
[mediaService getMediaURLFromVideoPressID:self.videopressGUID inBlog:self.blog success:^(NSString *videoURL, NSString *posterURL) {
[mediaService getMetadataFromVideoPressID: self.videopressGUID inBlog:self.blog success:^(RemoteVideoPressVideo *metadata) {
// Let see if can create an asset with this url
AVURLAsset *asset = [AVURLAsset assetWithURL:[NSURL URLWithString:videoURL]];
NSURL *originalURL = metadata.originalURL;
if (!originalURL) {
NSString *errorMessage = NSLocalizedString(@"Selected media is unavailable.", @"Error message when user tries a no longer existent video media object.");
completionHandler(nil, [self errorWithMessage:errorMessage]);
return;
}
NSURL *videoURL = [metadata getURLWithToken:originalURL] ?: originalURL;
AVURLAsset *asset = [AVURLAsset assetWithURL:videoURL];
if (!asset) {
NSString *errorMessage = NSLocalizedString(@"Selected media is unavailable.", @"Error message when user tries a no longer existent video media object.");
completionHandler(nil, [self errorWithMessage:errorMessage]);
Expand Down
18 changes: 11 additions & 7 deletions WordPress/Classes/Services/MediaService.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@


@class Media;
@class RemoteVideoPressVideo;
@class Blog;
@class AbstractPost;
@protocol ExportableAsset;
Expand Down Expand Up @@ -120,17 +121,20 @@ typedef NS_ERROR_ENUM(MediaServiceErrorDomain, MediaServiceError) {
failure:(nullable void (^)(void))failure;

/**
* Obtains the video url and poster image url for the video with the videoPressID
* Retrieves the metadata of a VideoPress video.
*
* @param videoPressID ID of video in VideoPress
* @param blog blog to use to access video references
* @param success return block if videopress info is found
* @param failure return block if not information found.
* The metadata parameters can be found in the API reference:
* https://developer.wordpress.com/docs/api/1.1/get/videos/%24guid/
*
* @param videoPressID ID of the video in VideoPress.
* @param success a block to be executed when the metadata is fetched successfully.
* @param failure a block to be executed when the metadata can't be fetched.
*/
- (void)getMediaURLFromVideoPressID:(nonnull NSString *)videoPressID
- (void)getMetadataFromVideoPressID:(nonnull NSString *)videoPressID
inBlog:(nonnull Blog *)blog
success:(nullable void (^)(NSString * _Nonnull videoURL, NSString * _Nullable posterURL))success
success:(nullable void (^)(RemoteVideoPressVideo * _Nonnull metadata))success
failure:(nullable void (^)(NSError * _Nonnull error))failure;

/**
* Sync all Media objects from the server to local database

Expand Down
8 changes: 4 additions & 4 deletions WordPress/Classes/Services/MediaService.m
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,15 @@ - (void) getMediaWithID:(NSNumber *) mediaID inBlog:(Blog *) blog
}];
}

- (void)getMediaURLFromVideoPressID:(NSString *)videoPressID
- (void)getMetadataFromVideoPressID:(NSString *)videoPressID
inBlog:(Blog *)blog
success:(void (^)(NSString *videoURL, NSString *posterURL))success
success:(void (^)(RemoteVideoPressVideo *metadata))success
failure:(void (^)(NSError *error))failure
{
id<MediaServiceRemote> remote = [self remoteForBlog:blog];
[remote getVideoURLFromVideoPressID:videoPressID success:^(NSURL *videoURL, NSURL *posterURL) {
[remote getMetadataFromVideoPressID:videoPressID isSitePrivate:blog.isPrivate success:^(RemoteVideoPressVideo *metadata) {
if (success) {
success(videoURL.absoluteString, posterURL.absoluteString);
success(metadata);
}
} failure:^(NSError * error) {
if (failure) {
Expand Down
4 changes: 2 additions & 2 deletions WordPress/Classes/Services/PostCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ class PostCoordinator: NSObject {
switch result {
case .failure:
handleSingleMediaFailure()
case .success(let value):
media.remoteURL = value.videoURL.absoluteString
case .success(let videoURL):
media.remoteURL = videoURL.absoluteString
successHandler()
}
}
Expand Down
4 changes: 2 additions & 2 deletions WordPress/Classes/Services/Stories/StoryMediaLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ class StoryMediaLoader {
self.downloadTasks.append(task)
}
case .video:
EditorMediaUtility.fetchRemoteVideoURL(for: media, in: post) { [weak self] result in
EditorMediaUtility.fetchRemoteVideoURL(for: media, in: post, withToken: true) { [weak self] result in
switch result {
case .success((let videoURL, _)):
case .success((let videoURL)):
self?.queue.async {
self?.results[idx] = (CameraSegment.video(videoURL, nil), nil)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2845,10 +2845,12 @@ extension AztecPostViewController {
let videoPressID = videoSrcURL.host {
// It's videoPress video so let's fetch the information for the video
let mediaService = MediaService(managedObjectContext: ContextManager.sharedInstance().mainContext)
mediaService.getMediaURL(fromVideoPressID: videoPressID, in: self.post.blog, success: { (videoURLString, posterURLString) in
videoAttachment.updateURL(URL(string: videoURLString))
if let validPosterURLString = posterURLString, let posterURL = URL(string: validPosterURLString) {
videoAttachment.posterURL = posterURL
mediaService.getMetadataFromVideoPressID(videoPressID, in: self.post.blog, success: { (metadata) in
if let originalURL = metadata.originalURL {
videoAttachment.updateURL(metadata.getURLWithToken(url: originalURL) ?? originalURL)
}
if let posterURL = metadata.posterURL {
videoAttachment.posterURL = metadata.getURLWithToken(url: posterURL) ?? posterURL
}
self.richTextView.refresh(videoAttachment)
}, failure: { (error) in
Expand Down Expand Up @@ -3021,20 +3023,21 @@ extension AztecPostViewController {
}
// It's videoPress video so let's fetch the information for the video
let mediaService = MediaService(managedObjectContext: ContextManager.sharedInstance().mainContext)
mediaService.getMediaURL(fromVideoPressID: videoPressID, in: self.post.blog, success: { [weak self] (videoURLString, posterURLString) in
mediaService.getMetadataFromVideoPressID(videoPressID, in: self.post.blog, success: { [weak self] (metadata) in
guard let `self` = self else {
return
}
guard let videoURL = URL(string: videoURLString) else {
guard let originalURL = metadata.originalURL else {
self.displayUnableToPlayVideoAlert()
return
}
videoAttachment.updateURL(videoURL)
if let validPosterURLString = posterURLString, let posterURL = URL(string: validPosterURLString) {
videoAttachment.posterURL = posterURL
let newVideoURL = metadata.getURLWithToken(url: originalURL) ?? originalURL
videoAttachment.updateURL(newVideoURL)
if let posterURL = metadata.posterURL {
videoAttachment.posterURL = metadata.getURLWithToken(url: posterURL) ?? posterURL
}
self.richTextView.refresh(videoAttachment)
self.displayVideoPlayer(for: videoURL)
self.displayVideoPlayer(for: newVideoURL)
}, failure: { [weak self] (error) in
self?.displayUnableToPlayVideoAlert()
DDLogError("Unable to find information for VideoPress video with ID = \(videoPressID). Details: \(error.localizedDescription)")
Expand Down
58 changes: 39 additions & 19 deletions WordPress/Classes/ViewRelated/Gutenberg/EditorMediaUtility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ final class AuthenticatedImageDownload: AsyncOperation {
}

class EditorMediaUtility {
private static let InternalInconsistencyError = NSError(domain: NSExceptionName.internalInconsistencyException.rawValue, code: 0)

private struct Constants {
static let placeholderDocumentLink = URL(string: "documentUploading://")!
Expand Down Expand Up @@ -153,31 +154,50 @@ class EditorMediaUtility {
return imageDownload
}

static func fetchRemoteVideoURL(for media: Media, in post: AbstractPost, completion: @escaping ( Result<(videoURL: URL, posterURL: URL?), Error> ) -> Void) {
guard let videoPressID = media.videopressGUID else {
//the site can be a self-hosted site if there's no videopressGUID
if let videoURLString = media.remoteURL,
let videoURL = URL(string: videoURLString) {
completion(Result.success((videoURL: videoURL, posterURL: nil)))
} else {
static func fetchRemoteVideoURL(for media: Media, in post: AbstractPost, withToken: Bool = false, completion: @escaping ( Result<(URL), Error> ) -> Void) {
// Return the attachment url it it's not a VideoPress video
if media.videopressGUID == nil {
guard let videoURLString = media.remoteURL, let videoURL = URL(string: videoURLString) else {
DDLogError("Unable to find remote video URL for video with upload ID = \(media.uploadID).")
completion(Result.failure(NSError()))
completion(Result.failure(InternalInconsistencyError))
return
}
completion(Result.success(videoURL))
}
else {
fetchVideoPressMetadata(for: media, in: post) { result in
switch result {
case .success((let metadata)):
guard let originalURL = metadata.originalURL else {
DDLogError("Failed getting original URL for media with upload ID: \(media.uploadID)")
completion(Result.failure(InternalInconsistencyError))
return
}
if withToken {
completion(Result.success(metadata.getURLWithToken(url: originalURL) ?? originalURL))
}
else {
completion(Result.success(originalURL))
}
case .failure(let error):
completion(Result.failure(error))
}
}
}
}

static func fetchVideoPressMetadata(for media: Media, in post: AbstractPost, completion: @escaping ( Result<(RemoteVideoPressVideo), Error> ) -> Void) {
guard let videoPressID = media.videopressGUID else {
DDLogError("Unable to find metadata for video with upload ID = \(media.uploadID).")
completion(Result.failure(InternalInconsistencyError))
return
}

let mediaService = MediaService(managedObjectContext: ContextManager.sharedInstance().mainContext)
mediaService.getMediaURL(fromVideoPressID: videoPressID, in: post.blog, success: { (videoURLString, posterURLString) in
guard let videoURL = URL(string: videoURLString) else {
completion(Result.failure(NSError()))
return
}
var posterURL: URL?
if let validPosterURLString = posterURLString, let url = URL(string: validPosterURLString) {
posterURL = url
}
completion(Result.success((videoURL: videoURL, posterURL: posterURL)))
mediaService.getMetadataFromVideoPressID(videoPressID, in: post.blog, success: { (metadata) in
completion(Result.success(metadata))
}, failure: { (error) in
DDLogError("Unable to find information for VideoPress video with ID = \(videoPressID). Details: \(error.localizedDescription)")
DDLogError("Unable to find metadata for VideoPress video with ID = \(videoPressID). Details: \(error.localizedDescription)")
completion(Result.failure(error))
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,20 +259,25 @@ class GutenbergMediaInserterHelper: NSObject {
}
switch media.mediaType {
case .video:
EditorMediaUtility.fetchRemoteVideoURL(for: media, in: post) { [weak self] (result) in
guard let strongSelf = self else {
return
}
switch result {
case .failure:
strongSelf.gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .failed, progress: 0, url: nil, serverID: nil)
case .success(let value):
var metadata: [String: Any] = [:]
if let videopressGUID = media.videopressGUID {
metadata["videopressGUID"] = videopressGUID
// Fetch metadata when is a VideoPress video
if media.videopressGUID != nil {
EditorMediaUtility.fetchVideoPressMetadata(for: media, in: post) { [weak self] (result) in
guard let strongSelf = self else {
return
}
switch result {
case .failure:
strongSelf.gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .failed, progress: 0, url: nil, serverID: nil)
case .success(let metadata):
strongSelf.gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .succeeded, progress: 1, url: metadata.originalURL, serverID: mediaServerID, metadata: metadata.asDictionary())
}
strongSelf.gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .succeeded, progress: 1, url: value.videoURL, serverID: mediaServerID, metadata: metadata)
}
} else {
guard let remoteURLString = media.remoteURL, let remoteURL = URL(string: remoteURLString) else {
gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .failed, progress: 0, url: nil, serverID: nil)
return
}
gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .succeeded, progress: 1, url: remoteURL, serverID: mediaServerID)
}
default:
gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .succeeded, progress: 1, url: url, serverID: mediaServerID)
Expand Down