Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Modernized networking architecture with Swift's Combine framework. #394

Merged
merged 6 commits into from
Nov 20, 2024
Merged

Modernized networking architecture with Swift's Combine framework. #394

merged 6 commits into from
Nov 20, 2024

Conversation

JaysonHahn
Copy link
Member

Overview

Modernized networking architecture with Swift's Combine framework.

Changes Made

Change 1: Network Reachability

  • Switched from the legacy reachability approach to Swift's Network module, enabling more accurate detection of network status, including Wi-Fi, cellular, or no connection.
  • Enhanced connectivity management across the app by adopting a modern, more reliable solution.

Change 2: Networking Refactor with Combine

  • Replaced the outdated FutureNova networking library with Combine, improving support for live data handling, such as real-time transit updates.
  • Enhanced error handling throughout the networking stack, leading to better stability and faster response to connectivity issues.

Change 3: Search Manager Optimization

  • Refactored the SearchManager to use Combine for debouncing search queries, ensuring that only the last query after a set delay is sent to the backend, reducing API costs.
  • Improved overall search performance by avoiding redundant queries and managing network resources more efficiently.

Change 4: Trip Sharing and Deep Linking

  • Implemented trip sharing via deep links, allowing users to share locations and route options through browser links that open directly in the app, enhancing user experience and convenience.

Test Coverage

  • Tested reachability scenarios across different network states (Wi-Fi, cellular, offline).
  • Verified real-time data updates with Combine in the transit feature.
  • Ensured that debounced searches function correctly in the SearchManager.
  • Confirmed deep link handling for shared trips, including link opening and in-app routing.

Next Steps (optional)

  • Monitor performance and change some functionality as needed.
  • Implement more Errors in Error handling when the backend updates how it does error handling

Screenshots (optional)

Simulator Screenshot - iPhone 16 - 2024-10-23 at 22 50 38

TODO
Before After
TODO
Before After
TODO

MrPeterss
MrPeterss requested changes Oct 25, 2024
View reviewed changes
Copy link

@MrPeterss MrPeterss left a comment

Choose a reason for hiding this comment

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

Really nice job on the refactor. I really like some of the design choices you made. I only have nit comments. Just request me again when you address them!

TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Controllers/HomeMapViewController.swift Show resolved Hide resolved
TCAT/Controllers/HomeOptionsCardViewController+Extensions.swift Show resolved Hide resolved
TCAT/Services/Network/ApiEndpoint.swift Show resolved Hide resolved
TCAT/Services/Network/ApiEndpoint.swift Show resolved Hide resolved
TCAT/Services/Network/ApiErrorHandler.swift Show resolved Hide resolved
TCAT/Services/Network/NetworkManager.swift Show resolved Hide resolved
TCAT/Services/Network/NetworkMonitor.swift Show resolved Hide resolved
angelinaa-chen
angelinaa-chen approved these changes Oct 26, 2024
View reviewed changes
Copy link
Contributor

@angelinaa-chen angelinaa-chen left a comment

Choose a reason for hiding this comment

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

Looks good Jayson!! I just added a couple very very minuscule comments but otherwise everything else looks good to go. Glad Transit's networking is getting a new makeover (:

TCAT/Controllers/RouteDetail+ContentViewController.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Services/Network/NetworkManager.swift Outdated Show resolved Hide resolved
rs929
rs929 approved these changes Oct 27, 2024
View reviewed changes
Copy link
Contributor

@rs929 rs929 left a comment

Choose a reason for hiding this comment

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

Great PR, absolute 🐐!

I really appreciate you taking the initiative to refactor networking, its quite a daunting task and you killed it!

Mainly just a few styling issues but otherwise amazing!

TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Base/AppDelegate.swift Outdated Show resolved Hide resolved
TCAT/Services/Network/ApiEndpoint.swift Show resolved Hide resolved
TCAT/Services/Network/ApiEndpoint.swift Show resolved Hide resolved
TCAT/Services/Network/ApiEndpoint.swift Outdated Show resolved Hide resolved
TCAT/Services/Transit/TransitProvider.swift Show resolved Hide resolved
TCAT/Services/Transit/TransitProvider.swift Show resolved Hide resolved
@JaysonHahn JaysonHahn requested review from MrPeterss and rs929 November 2, 2024 20:48
MrPeterss
MrPeterss approved these changes Nov 2, 2024
View reviewed changes
Copy link

@MrPeterss MrPeterss left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks for the fixes

rs929
rs929 approved these changes Nov 3, 2024
View reviewed changes
Copy link
Contributor

@rs929 rs929 left a comment

Choose a reason for hiding this comment

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

Great Job!! The swiftLint thing is weird. Usually we don't want extra parethese where we don't need them. I'll look into it but you can keep it the same for now.

LGTM!

@cuappdev cuappdev deleted a comment from rs929 Nov 20, 2024
@JaysonHahn JaysonHahn merged commit 4851103 into master Nov 20, 2024
@JaysonHahn JaysonHahn deleted the Jayson/Networking branch November 20, 2024 23:17
rs929 added a commit that referenced this pull request Nov 20, 2024
@JaysonHahn @vinnie4k
@cindy-x-liang @angelinaa-chen @rs929
Release 2.0.4 Build 22 (#400)
7dc4031 
* Remove codegen build phase

* add route filtering

* deleted debugging stuff

* Added firebase messaging to iOS

* addressing richie's comments

* Show delay times for start & end, needs double checked with backend

* Modified / improved route UI

* Reduced redundant code; cleaned up files

* Changed location marker image, reorganizec code

* Minor comment changes

* Addressed Peter's comments

* Fixed minor issue

* Addressed Richie's comments p1

* Addressed Richie's comments p2

* Addressed Richie's comments p3

* Release (#398)

* Remove codegen build phase

* add route filtering

* deleted debugging stuff

* Show delay times for start & end, needs double checked with backend

* Added firebase messaging to iOS

* addressing richie's comments

* Modified / improved route UI

* Reduced redundant code; cleaned up files

* Changed location marker image, reorganizec code

* Minor comment changes

* Addressed Peter's comments

* Fixed minor issue

* Addressed Richie's comments

* Release TestFlight 2.0.3 Build 19 (#399)

* Remove codegen build phase

* add route filtering

* deleted debugging stuff

* Added firebase messaging to iOS

* addressing richie's comments

* Show delay times for start & end, needs double checked with backend

* Modified / improved route UI

* Reduced redundant code; cleaned up files

* Changed location marker image, reorganizec code

* Minor comment changes

* Addressed Peter's comments

* Fixed minor issue

* Addressed Richie's comments p1

* Addressed Richie's comments p2

* Addressed Richie's comments p3

* Incremented Build and Version

---------

Co-authored-by: Vin Bui <vdb23@cornell.edu>
Co-authored-by: Vin Bui <75594943+vinnie4k@users.noreply.github.com>
Co-authored-by: cindy-x-liang <67083541+cindy-x-liang@users.noreply.github.com>
Co-authored-by: Angelina Chen <angelina.chhen@gmail.com>

---------

Co-authored-by: Vin Bui <75594943+vinnie4k@users.noreply.github.com>
Co-authored-by: Vin Bui <vdb23@cornell.edu>
Co-authored-by: cindy-x-liang <67083541+cindy-x-liang@users.noreply.github.com>
Co-authored-by: Angelina Chen <angelina.chhen@gmail.com>

* Modernized networking architecture with Swift's Combine framework. (#394)

* Initial Networking

* Finish network refactor

* Fix code styling

* Merge branch 'master' into Jayson/Networking

* update version

---------

Co-authored-by: Vin Bui <vdb23@cornell.edu>
Co-authored-by: Vin Bui <75594943+vinnie4k@users.noreply.github.com>
Co-authored-by: cindy-x-liang <67083541+cindy-x-liang@users.noreply.github.com>
Co-authored-by: Angelina Chen <angelina.chhen@gmail.com>
Co-authored-by: Richie Sun <105038960+rs929@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrPeterss MrPeterss MrPeterss approved these changes

@rs929 rs929 rs929 approved these changes

@angelinaa-chen angelinaa-chen angelinaa-chen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JaysonHahn @MrPeterss @rs929 @angelinaa-chen