[NEW] Use REST API to upload files with fallback to older servers #1058

cardoso · 2017-12-08T14:56:12Z

@RocketChat/ios

cardoso · 2017-12-08T14:59:06Z

This is not working yet...

I'm receiving this error from the server:

{
    "error" : "File required", 
    "success" : false
}

For reference:
API
Busboy

rafaelks · 2017-12-08T15:00:09Z

Rocket.Chat/Managers/Uploader/UploadManager.swift

@@ -76,6 +76,20 @@ class UploadManager {

    // swiftlint:disable function_body_length
    func uploadToUFSFile(store: String, file: FileUpload, subscription: Subscription, progress: UploadProgressBlock, completion: @escaping UploadCompletionBlock) {
+
+        if let api = API.current() {


Shouldn't API.current() always return something valid? 🤔

Not how it's working currently. I use the Auth object from AuthManager.isAuthenticated() to build it.

rafaelks · 2017-12-08T15:06:47Z

@cardoso Take a look here in the code below, we're using a different Content-Type for the file and the order of the parameters are also different.

Rocket.Chat.iOS/Rocket.Chat/Managers/Uploader/UploadManager.swift

Lines 42 to 60 in ce206a6

    
           let boundary = String.random() 
        
           let contentType = "multipart/form-data; boundary=\(boundary)" 
        
           request.addValue(contentType, forHTTPHeaderField: "Content-Type") 
        
           var data = Data() 
        
           data.append("\r\n--\(boundary)\r\n".data(using: .utf8) ?? Data()) 
        
           for postData in formData?.array ?? [] { 
        
               guard let key = postData["name"].string else { continue } 
        
               guard let value = postData["value"].string else { continue } 
        
               data.append("Content-Disposition: form-data; name=\"\(key)\"\r\n\r\n".data(using: .utf8) ?? Data()) 
        
               data.append(value.data(using: .utf8) ?? Data()) 
        
               data.append("\r\n--\(boundary)\r\n".data(using: .utf8) ?? Data()) 
        
           } 
        
           data.append("Content-Disposition: form-data; name=\"file\"\r\n".data(using: .utf8) ?? Data()) 
        
           data.append("Content-Type: application/octet-stream\r\n\r\n".data(using: .utf8) ?? Data()) 
        
           data.append(file.data) 
        
           data.append("\r\n--\(boundary)--\r\n".data(using: .utf8) ?? Data())

codecov · 2017-12-08T15:35:27Z

Codecov Report

Merging #1058 into develop will increase coverage by 4.2%.
The diff coverage is 34.28%.

@@            Coverage Diff             @@
##           develop    #1058     +/-   ##
==========================================
+ Coverage    39.86%   44.06%   +4.2%     
==========================================
  Files          254      257      +3     
  Lines        14307    14419    +112     
  Branches       742      791     +49     
==========================================
+ Hits          5703     6354    +651     
+ Misses        8144     7599    -545     
- Partials       460      466      +6

Impacted Files	Coverage Δ
...Chat/API/Requests/Message/SendMessageRequest.swift	`31.25% <ø> (ø)`	⬆️
Rocket.Chat/Helpers/Alert.swift	`0% <0%> (ø)`	⬆️
Rocket.Chat/API/Clients/UploadClient.swift	`0% <0%> (ø)`
...Chat/Controllers/Chat/ChatControllerUploader.swift	`0% <0%> (ø)`	⬆️
Rocket.Chat/API/APIRequest.swift	`86.53% <100%> (+5.76%)`	⬆️
...Tests/API/Requests/Message/UploadRequestSpec.swift	`61.53% <61.53%> (ø)`
Rocket.Chat/Extensions/DataExtension.swift	`25% <85.71%> (+25%)`	⬆️
...cket.Chat/API/Requests/Message/UploadRequest.swift	`91.66% <91.66%> (ø)`
...Controllers/Auth/ConnectServerViewController.swift	`34.16% <0%> (-2.49%)`	⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe015ce...1196d5f. Read the comment docs.

houndci-bot · 2017-12-12T00:47:04Z

Rocket.Chat/Managers/Uploader/UploadManager.swift

+                completion(nil, false)
+            }, errored: { error in
+                if case .version = error {
+                    // TODO: Remove Upload fallback after Rocket.Chat 1.0


Todo Violation: TODOs should be avoided (Remove Upload fallback after R...). (todo)

cardoso · 2017-12-12T10:54:17Z

@rafaelks ready for review 👍 thanks for the help!!

rafaelks · 2017-12-12T12:12:02Z

Rocket.Chat/Managers/Uploader/UploadManager.swift

-                            completion(response, true)
-                            return
-                        }
+                    completion(nil, true)


Can we send the error to the completion block? I think would be interesting to show the error message here, example: file limit is 2MB.

cardoso · 2017-12-13T00:15:10Z

Blocked by RocketChat/Rocket.Chat#9096

… into feat/upload_http.1055

houndci-bot · 2017-12-14T14:54:10Z

Rocket.Chat/API/Clients/UploadClient.swift

+            completion?()
+        }, errored: { error in
+            if case .version = error {
+                // TODO: Remove Upload fallback after Rocket.Chat 1.0


Todo Violation: TODOs should be avoided (Remove Upload fallback after R...). (todo)

cardoso · 2017-12-14T14:56:45Z

@rafaelks now it's showing error messages (unblocked by RocketChat/Rocket.Chat#9108)

rafaelks · 2017-12-15T11:30:47Z

Rocket.Chat/API/Clients/UploadClient.swift

+        })
+    }
+
+    func deletePushToken(token: String? = PushManager.getDeviceToken()) {


Why is deletePushToken method here? 🤔

No good reason 😊 thanks for spotting it 😂

rafaelks · 2017-12-15T11:31:43Z

Rocket.Chat/API/Clients/UploadClient.swift

+
+        api.fetch(req, succeeded: { result in
+            if let error = result.error {
+                Alert(key: "alert.upload_error").withMessage(error).present()


status code 2xx with error (I know it's API's fault)? 😂

🤷‍♀️

Upload files via REST (not working yet)

5f9061c

cardoso added 🎉 feature 📡 networking labels Dec 8, 2017

rafaelks reviewed Dec 8, 2017

View reviewed changes

Remove extra line break (still not working)

15fa368

rafaelks and others added 4 commits December 11, 2017 13:01

Make some code adjutments

defe6ff

Fixed breaking likes

23a301f

Send mimetype

ac0b7d2

Upload fallback

09086ff

houndci-bot reviewed Dec 12, 2017

View reviewed changes

cardoso added 2 commits December 11, 2017 21:57

Remove Content-Length from request headers

b53fa3e

add UploadRequestSpec

c1788a9

cardoso changed the title ~~[WIP][NEW] Use REST API to upload files with fallback to older servers~~ [NEW] Use REST API to upload files with fallback to older servers Dec 12, 2017

rafaelks reviewed Dec 12, 2017

View reviewed changes

cardoso added the 🚫 blocked: other issue label Dec 13, 2017

cardoso added 2 commits December 14, 2017 08:25

Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat.iOS…

a50d757

… into feat/upload_http.1055

Show error when upload failed (new REST method)

672ffeb

cardoso removed the 🚫 blocked: other issue label Dec 14, 2017

houndci-bot reviewed Dec 14, 2017

View reviewed changes

rafaelks suggested changes Dec 15, 2017

View reviewed changes

Remove duplicated code

1196d5f

rafaelks approved these changes Dec 15, 2017

View reviewed changes

rafaelks merged commit 27d11bf into develop Dec 18, 2017

rafaelks deleted the feat/upload_http.1055 branch December 18, 2017 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NEW] Use REST API to upload files with fallback to older servers #1058

[NEW] Use REST API to upload files with fallback to older servers #1058

cardoso commented Dec 8, 2017

cardoso commented Dec 8, 2017

rafaelks Dec 8, 2017

cardoso Dec 8, 2017

rafaelks commented Dec 8, 2017

codecov bot commented Dec 8, 2017 •

edited

Loading

houndci-bot Dec 12, 2017

cardoso commented Dec 12, 2017 •

edited

Loading

rafaelks Dec 12, 2017

cardoso commented Dec 13, 2017

houndci-bot Dec 14, 2017

cardoso commented Dec 14, 2017 •

edited

Loading

rafaelks Dec 15, 2017

cardoso Dec 15, 2017 •

edited

Loading

rafaelks Dec 15, 2017

cardoso Dec 15, 2017

[NEW] Use REST API to upload files with fallback to older servers #1058

[NEW] Use REST API to upload files with fallback to older servers #1058

Conversation

cardoso commented Dec 8, 2017

cardoso commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaelks commented Dec 8, 2017

codecov bot commented Dec 8, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

cardoso commented Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

cardoso commented Dec 13, 2017

Choose a reason for hiding this comment

cardoso commented Dec 14, 2017 • edited Loading

Choose a reason for hiding this comment

cardoso Dec 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 8, 2017 •

edited

Loading

cardoso commented Dec 12, 2017 •

edited

Loading

cardoso commented Dec 14, 2017 •

edited

Loading

cardoso Dec 15, 2017 •

edited

Loading