Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

v4 Refactor Notice #653

Merged
merged 32 commits into from
Jan 2, 2020
Merged

v4 Refactor Notice #653

merged 32 commits into from
Jan 2, 2020

Conversation

acmacalister
Copy link
Collaborator

This branch is currently a major refactor of the codebase into more modern Swift. It is protocol driven to make things more easily extensible. The code should be stable and ready for testing (it still passes the whole Autobahn test suite). It also makes use of the new Network.framework Below are some of the TODOs left:

  • Test WatchOS/TVOS support. The original Core Foundation APIs have been updated to use the Network.framework or Foundation APIs as a fallback. Testing is still needed to see if this will effect WatchOS. In the past, WatchOS didn't have the Foundation APIs available, meaning we were forced to drop down to the CoreFoundation APIs. If this is still the case, Another Transport will need to be implemented to bring this functionality back.

  • Finish the tests. Work has been done to replicate all the existing Autobahn test suite. Examples can be seen in the Tests directory. We would like to have all 500+ base cases tests so CI builds could easily test PRs for compatibility. Additional tests for the Certificate/Key Pinning are also planned. This is a huge improvement over the current testing cycle which is generally done manually.

  • Experimental server support. With the addition of the Network.framework, WebSocket server support is much easier and being explored. (See the Server folder for more information)

  • Test Linux support.

Any testing or feedback are appreciated. If any of the above TODOs sound interesting to you, feel free to comment on this PR and we can help get you setup to contribute on those.

This was referenced Jun 7, 2019
@daltoniam
Copy link
Owner

I agree this is an exciting development. After 5 years of development on Starscream, we are getting a native WebSocket API. Excitement and sentiment aside, I think we should add the native API support if it is available. It is going to be the most work, but I don't see a reason to not use the native APIs "under the hood" when available, especially since we are already refactoring everything. I think this will allow for the easiest transition from Starscream to the native APIs as it makes sense and for easy support of the different API availability on each OS version.

@fassko
Copy link
Collaborator

fassko commented Jun 17, 2019

My 2 cents would be to release current branch now and start working to support native APIs behind the scene then. Basically plan nr. 2 from @acmacalister comment #653 (comment)

I'm actually very happy (I was in WWDC session myself) that Apple finally is bringing native support for WebSockets in Network.framework and URLSession. We need to support -1 or sometimes -2 iOS version for our apps so it will take to adapt it fully and in either way Starscream will be used at least 2 years from now. But assume it will be even longer.

@rudyrichter
Copy link

I agree with that, we have some changes we'd like to submit that we're holding off on until this PR is merged.

@rudyrichter
Copy link

@fassko any movement here?

@fassko
Copy link
Collaborator

fassko commented Aug 7, 2019

@rudyrichter sadly no, but soon :)

@WyattMufson
Copy link

@fassko any guess when this will be finished? Thanks

@daltoniam
Copy link
Owner

@acmacalister and @fassko I think I finished up everything so we can (finally) get this merged!

@fassko
Copy link
Collaborator

fassko commented Oct 1, 2019

Thanks @daltoniam I will check this in coming days! Sorry for delays. :(

@fassko
Copy link
Collaborator

fassko commented Oct 1, 2019

@daltoniam can you take a look at unit tests? I can't build them.
image

@Marcocanc
Copy link

This is amazing.
Just saw that the URLSessionWebSocketTask stuff is commented out though.
Any idea when this will be merged?

@sjmueller
Copy link

I agree, excited that starscream will use the native URLSessionWebSocketTask api. Can't wait to see this merged.

@daltoniam daltoniam merged commit 6b27425 into master Jan 2, 2020
@daltoniam daltoniam changed the title v4 Refactor Notice (WIP) v4 Refactor Notice Jan 2, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants