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.

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

Networking Refactor - PR Series #195

Merged
merged 7 commits into from
Sep 6, 2023
Merged

Networking Refactor - PR Series #195

merged 7 commits into from
Sep 6, 2023

Conversation

scannillo
Copy link
Collaborator

@scannillo scannillo commented Sep 5, 2023

Reason for changes

These changes have already been reviewed in this series of PRs. I created this PR solely to confirm CI passes.

⚠️ Do not squash and merge

Summary

These PRs re-architect our networking layer to allow for re-use between GraphQL and REST request implementation. We can now use the same underlying HTTP layer for each request, in addition to a single implementation of Apple's URLSession, URLRequest, JSONDecoder, & JSONEncoder.

This reduces duplication, and increases testability of each layer through removing heavy logic that previously lived in protocol extensions. This also unifies our APIs for making a REST & GraphQL request from the feature client level.

Before After
PPCP iOS Networking Before PPCP iOS Networking After

Next Steps

We need to create JIRAs for the following tasks:

  • Renaming APIClient --> NetworkingClient
  • Creating a wrapper/ custom JSONEncoder layer
  • Re-organize file structure in Sources
  • Decide on consistent naming convention for model (struct) request types

Checklist

  • Added a changelog entry

Authors

@scannillo

scannillo and others added 6 commits August 16, 2023 09:36
…ssAPI.swift` layers (#177)

- Add `TrackingEventsAPI.swift` & `CheckoutOrdersAPI.swift`
- Remove usage of `APIRequest` protocol in favor of inline `RESTRequest` object creation
- Add `HTTPResponse` type
- Set the stage for having a GraphQL & REST requests use same same underlying `HTTP` layer
…plementation (#182)

- Add `fetch(request: GraphQLRequest)` method to `APIClient`
   - This class now has only 2 public functions, an equivalent for REST & GraphQL
   - Unifies REST & GraphQL requests to use the same underlying HTTP, URLRequest, and URLSession implementations
- Remove `GraphQLClient` and other no-longer-used GraphQL files
- Rename `GraphQLQueryResponse` --> `GraphQLHTTPResponse`
- Add GraphQL response parsing ability to `HTTPResponseParser`
- Move Post body encoding from `APIClient`
   - Require `Encodable` type in `RESTRequest` & `GraphQLRequest`s instead of `Data`
- Add `VaultPaymentTokensAPI` layer
- Tests
    - Remove `GraphQLClient_Tests` now that `GraphQLClient` is gone
    - Update `HTTPResponseParser_Tests`
 - Remove `APIRequest` protocol
- Update `TrackingEventsAPI` for mock `APIClient` injection
- Update mocks
    - `MockTrackingEventsAPI` (added), `MockAPIClient` (updated), `MockHTTP` (updated)
    - Use `captured` terminology on mocks
 - Fix unit tests `HTTP_Tests`, `APIClient_Tests`, `AnalyticsService_Tests` & add `TrackingEventsAPI_Tests`
- Add tests for:
    - `CheckoutOrdersAPI`
    - `VaultPaymentsTokenAPI`
- Update tests for: `CardClient` &  `ConfirmPaymentSourceRequest`
- Move mocks into `CardPaymentTests/Mocks` dir
- Add mocks for:
    - `CheckoutOrdersAPI` and `VaultPaymentTokensAPI` for use in `CardClient_Tests`
    - Repurpose `CardResponses` file of sample JSON responses to be raw `ConfirmPaymentSourceReponse` types
@scannillo scannillo requested a review from a team as a code owner September 5, 2023 23:18
@scannillo scannillo merged commit bb72d32 into main Sep 6, 2023
4 checks passed
@scannillo scannillo deleted the networking-refactor branch September 6, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant