[FIX] Enable support for Rocket hosted on subdirectory #1277

jpowie01 · 2018-02-16T21:42:34Z

@RocketChat/ios

Closes #900

These changes make sure that API Requests will follow HTTP protocol with applied subdirectory on which Rocket.Chat Server is hosted. Also, saves HTTP connection to the database instead of WebSocket one.

Note: It would be nice if anyone would like to check it out and even countinue work on this as I'm not familiar with Swift/iOS/Rocket.Chat.iOS at all. It's my proposal for changes :)

CLAassistant · 2018-02-16T21:42:41Z

All committers have signed the CLA.

cardoso

Thanks @jpowie01 !!

Let's see how we can fix this problem in the best way possible.

What server are you using to test this? Is it public?
I requested couple changes for now.

cardoso · 2018-02-16T21:57:23Z

Rocket.Chat/API/API.swift

@@ -40,15 +40,15 @@ class API: APIFetcher {
    var userId: String?

    convenience init?(host: String, version: Version = .zero) {
-        guard let url = URL(string: host) else {
+        guard let url = URL(string: host)?.httpServerURL() else {


We shouldn't make these changes here.

It would be better if we make this change where we are initializing the API object.

Take a look at APIExtensions.swift

cardoso · 2018-02-16T21:57:36Z

Rocket.Chat/API/API.swift

            return nil
        }

        self.init(host: url, version: version)
    }

    init(host: URL, version: Version = .zero) {
-        self.host = host
+        self.host = host.httpServerURL() ?? host


jpowie01 · 2018-02-17T13:34:15Z

Yes, my server is publicly available but I wouldn't like to share a link here. I can send you an URL to my instance via email if you want to test it on your own. I'll try to resolve above comments today. Thanks! :)

cardoso · 2018-02-17T14:09:58Z

@jpowie01 Great! You can send me the link at https://open.rocket.chat/direct/matheus.cardoso

houndci-bot · 2018-02-17T14:24:25Z

Rocket.ChatTests/Models/AuthSpec.swift

+
+        XCTAssertEqual(object.apiHost?.absoluteString, "https://team.rocket.chat/subdirectory/", "apiHost returns API Host correctly")
+    }
+


Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

houndci-bot · 2018-02-17T14:24:25Z

Rocket.ChatTests/Models/AuthSpec.swift

+
+        XCTAssertEqual(object.apiHost?.absoluteString, "https://team.rocket.chat/subdirectory/", "apiHost returns API Host correctly")
+    }
+


Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

houndci-bot · 2018-02-17T14:24:25Z

Rocket.ChatTests/Models/AuthSpec.swift

+
+        XCTAssertEqual(object.apiHost?.absoluteString, "https://team.rocket.chat/subdirectory/", "apiHost returns API Host correctly")
+    }
+


Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

…ocket.Chat.iOS into rocket_hosted_on_subdirectory

rafaelks

On my tests, the custom emojis of open.rocket.chat didn't load. Can you guys also reproduce the issue?

jpowie01 · 2018-02-25T11:56:29Z

You're right! Debugged, fixed & pushed to this PR :)

…ocket.Chat.iOS into rocket_hosted_on_subdirectory

…rocket_hosted_on_subdirectory

rafaelks · 2018-02-27T14:39:18Z

@jpowie01 Tests aren't passing... can you take a look again please? I tried re-run the CI but still didn't pass.

jpowie01 · 2018-02-27T22:16:22Z

Could you help me guys with reproduction? All tests passes on my local machine. Here are logs from above test taken from Xcode:

Test Suite 'InfoClientSpec' started at 2018-02-27 23:04:12.555
Test Case '-[Rocket_ChatTests.InfoClientSpec testFetchInfo]' started.
[WebSocket] http upgrade response (HTTP/1.1 101 Switching Protocols
Upgrade: websocket
Connection: Upgrade
Sec-WebSocket-Accept: oV6ru9aDJyVARjY/Scpa+utZvdc=

)
[WebSocket] wss://open.rocket.chat/websocket
 -  did connect
[WebSocket] wss://open.rocket.chat/websocket
 -  will send message: {
  "version" : "1",
  "id" : "l1zIiojoKZfCgqK4AJDbeGsY3eBLlCb4hwMPvenndXYtDin78a",
  "support" : [
    "1",
    "pre2",
    "pre1"
  ],
  "msg" : "connect"
}
[WebSocket] wss://open.rocket.chat/websocket
 - did receive JSON message:
{
  "server_id" : "0"
}
Msg is invalid: {
  "server_id" : "0"
}
[WebSocket] wss://open.rocket.chat/websocket
 - did receive JSON message:
{
  "session" : "eSoTTYayYXu7mc3G7",
  "msg" : "connected"
}
[WebSocket] wss://open.rocket.chat/websocket
 -  will send message: {
  "id" : "FqadlQU4SBntoIkwWrQqz0GfDYYXl1H4IVS7hP8Pz3maA1BEFZ",
  "method" : "public-settings\/get",
  "msg" : "method"
}
[WebSocket] wss://open.rocket.chat/websocket
 - did receive JSON message:
{
  "methods" : [
    "FqadlQU4SBntoIkwWrQqz0GfDYYXl1H4IVS7hP8Pz3maA1BEFZ"
  ],
  "msg" : "updated"
}
Test Case '-[Rocket_ChatTests.InfoClientSpec testFetchInfo]' passed (1.143 seconds).

Am I right that I'm lucky it ends just before the deadline (1.1 second)? I rerun tests a couple of times and it just works:

Executed 325 tests, with 0 failures (0 unexpected) in 6.920 (7.390) seconds

My setup: Xcode Version 9.0.1 (9A1004).

rafaelks · 2018-02-28T16:32:19Z

@cardoso The test is failing in a "wait()" method. Can you take a look please?

cardoso · 2018-02-28T23:40:36Z

It's most likely a random crash @rafaelks. I'll take a look.

rafaelks · 2018-03-02T12:38:12Z

@jpowie01 Just made some testing and looks like the loading of attachments doesn't work on servers with subdirectory.

Steps to reproduce:

Authenticate in a server with subdirectory;
Upload a file;
The file won't show in your app;
The fill will show on the web browser;

Can you fix this one also, please?

jpowie01 · 2018-03-04T19:36:21Z

Thanks for that! I'll look into this issue tomorrow. That's probably a problem with string concatenation instead of API usage for building URLs.

rafaelks · 2018-03-05T13:59:47Z

@jpowie01 Sure! Let me know when you're done, we're looking to merge this one for next release. 👍

jpowie01 · 2018-03-05T21:01:15Z

I've debugged it and... it's strange :/

iOS application points to this place if it would like to fetch an attachment (eg. an image):
https://{MY_INSTANCE}/{SUBDIR}/file-upload/{SOME_HASH}/{FILE_NAME}.jpeg

But... the file is actually available under this path:
https://{MY_INSTANCE}/{SUBDIR}/{SUBDIR}/file-upload/{SOME_HASH}/{FILE_NAME}

So... Sending attatchments in browser works because it points to a place with double subdirectory in path!

What's more - desktop (Mac) application points to a single subdirectory path (like an iOS app) which causes the same issues as described here.

Maybe my server settings are broken? But it would be strange as all other things seems to work like a charm. For me, it sounds like a bug on the server side... Do you have any other instance running on a subdirectory just for testing?

rafaelks · 2018-03-06T09:51:30Z

@jpowie01 Thanks for looking into it. Looks like there's a bug on upload. I'll try to investigate with the back-end guys to see what's going on.

rafaelks · 2018-03-06T16:31:35Z

@jpowie01 Yes, the issue is fixed already and will be released to next release: RocketChat/Rocket.Chat#10029.

I'll try your PR with this version of the server and I'm gonna merge it if that works fine.

Thank you!!!

jpowie01 · 2018-03-06T19:57:08Z

Cool, great! Thanks for confirmation! :)

jpowie01 · 2018-03-22T20:53:20Z

I've updated my server to use the latest version of Rocket.chat and it fixed issues with attachments :)

rafaelks

Looking good!!! Thank you!

filipealva · 2018-03-23T16:33:42Z

Rocket.Chat/API/APIRequest.swift

@@ -58,7 +58,7 @@ extension APIRequest {

    func request(for api: API, options: APIRequestOptions = .none) -> URLRequest? {
        var components = URLComponents(url: api.host, resolvingAgainstBaseURL: false)
-        components?.path = path
+        components?.path += path


@RocketChat/ios Guys, this += has introduced a bug, because we specify our paths as the following: /api/v1/me, and if we concatenate this path with the default path of an URLComponents it will result in a duplicated initial slash as the following: //api/v1/me which makes the request invalid.

It took me a while to discover what was going on because the result it is generating is not even handled as an error by the app, so it generated confusing behaviors such as making the app stuck on an endless loading in the auth screen, or even authenticating the user with a nil user 🕵️

#1415 fixes it

@filipealva Thank you! 👍

Enable support for Rocket hosted on subdirectory

0a158fb

jpowie01 mentioned this pull request Feb 16, 2018

[BUG][INVESTIGATE] The server URL will have a Bug with a path, like this: "https: 192.168.1.100/chat" #900

Closed

cardoso self-requested a review February 16, 2018 21:53

cardoso suggested changes Feb 16, 2018

View reviewed changes

jpowie01 added 2 commits February 17, 2018 15:13

Move changes from API to APIExtentions

cd26ef8

Add tests for Rocket hosted on subdirectory

482b2ed

houndci-bot reviewed Feb 17, 2018

View reviewed changes

jpowie01 and others added 2 commits February 17, 2018 18:01

Remove trailing whitespace in AuthSpec tests

4fd12e7

Merge branch 'rocket_hosted_on_subdirectory' of github.com:jpowie01/R…

e5edf54

…ocket.Chat.iOS into rocket_hosted_on_subdirectory

rafaelks suggested changes Feb 22, 2018

View reviewed changes

Fix custom Emojis

69f3d1c

rafaelks added 2 commits February 26, 2018 09:12

Merge branch 'rocket_hosted_on_subdirectory' of github.com:jpowie01/R…

0047bbb

…ocket.Chat.iOS into rocket_hosted_on_subdirectory

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

99d90ed

…rocket_hosted_on_subdirectory

rafaelks added 👍 improvement 🚧 wip: contributor labels Feb 26, 2018

rafaelks added this to the 2.1.0 milestone Feb 26, 2018

rafaelks added the 🚫 blocked: other issue label Mar 7, 2018

rafaelks self-assigned this Mar 12, 2018

rafaelks approved these changes Mar 23, 2018

View reviewed changes

rafaelks merged commit 57287a3 into RocketChat:develop Mar 23, 2018

filipealva reviewed Mar 23, 2018

View reviewed changes

rafaelks removed 🚫 blocked: other issue 🚧 wip: contributor labels Mar 27, 2018

jpowie01 mentioned this pull request Apr 1, 2018

[FIX] Build URL that may use subdirectory #1470

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] Enable support for Rocket hosted on subdirectory #1277

[FIX] Enable support for Rocket hosted on subdirectory #1277

jpowie01 commented Feb 16, 2018

CLAassistant commented Feb 16, 2018 •

edited

Loading

cardoso left a comment

cardoso Feb 16, 2018

jpowie01 Feb 17, 2018

cardoso Feb 16, 2018

jpowie01 Feb 17, 2018

jpowie01 commented Feb 17, 2018

cardoso commented Feb 17, 2018

houndci-bot Feb 17, 2018

houndci-bot Feb 17, 2018

houndci-bot Feb 17, 2018

rafaelks left a comment

jpowie01 commented Feb 25, 2018

rafaelks commented Feb 27, 2018

jpowie01 commented Feb 27, 2018

rafaelks commented Feb 28, 2018

cardoso commented Feb 28, 2018 •

edited

Loading

rafaelks commented Mar 2, 2018

jpowie01 commented Mar 4, 2018

rafaelks commented Mar 5, 2018

jpowie01 commented Mar 5, 2018

rafaelks commented Mar 6, 2018

rafaelks commented Mar 6, 2018

jpowie01 commented Mar 6, 2018

jpowie01 commented Mar 22, 2018

rafaelks left a comment

filipealva Mar 23, 2018 •

edited

Loading

filipealva Mar 23, 2018

rafaelks Mar 25, 2018


		XCTAssertEqual(object.apiHost?.absoluteString, "https://team.rocket.chat/subdirectory/", "apiHost returns API Host correctly")
		}

[FIX] Enable support for Rocket hosted on subdirectory #1277

[FIX] Enable support for Rocket hosted on subdirectory #1277

Conversation

jpowie01 commented Feb 16, 2018

CLAassistant commented Feb 16, 2018 • edited Loading

cardoso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpowie01 commented Feb 17, 2018

cardoso commented Feb 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaelks left a comment

Choose a reason for hiding this comment

jpowie01 commented Feb 25, 2018

rafaelks commented Feb 27, 2018

jpowie01 commented Feb 27, 2018

rafaelks commented Feb 28, 2018

cardoso commented Feb 28, 2018 • edited Loading

rafaelks commented Mar 2, 2018

jpowie01 commented Mar 4, 2018

rafaelks commented Mar 5, 2018

jpowie01 commented Mar 5, 2018

rafaelks commented Mar 6, 2018

rafaelks commented Mar 6, 2018

jpowie01 commented Mar 6, 2018

jpowie01 commented Mar 22, 2018

rafaelks left a comment

Choose a reason for hiding this comment

filipealva Mar 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Feb 16, 2018 •

edited

Loading

cardoso commented Feb 28, 2018 •

edited

Loading

filipealva Mar 23, 2018 •

edited

Loading