Malware protection 2: refactor APIClient #1092
Conversation
There was a problem hiding this comment.
LGTM. Just left a comment on cache policies and timeouts. I've noticed that the current production deployment has a very long cache policy of 1 year, which causes some issues right now in internal.
I don't think it's worth changing the cache policy on the client though, as I'm working on a task to fix the cache headers on the backend separately. Might be good to just add a stricter timeout on /matches calls.
| } | ||
| return nil | ||
| public func matches(forHashPrefix hashPrefix: String) async throws -> APIClient.Response.Matches { | ||
| let result = try await load(.matches(hashPrefix: hashPrefix)) |
There was a problem hiding this comment.
Can we pass a shorter timeout to the /matches endpoint? This could block navigation so we should favour navigation loading if the backend is degraded. On Android we're looking at a maximum 1 second timeout for this request. The other requests can be heavier but they are background tasks anyway so not as risky.
There was a problem hiding this comment.
Added parametrized Timeout
| public func load<Request: APIRequestProtocol>(_ requestConfig: Request) async throws -> Request.ResponseType { | ||
| let requestType = requestConfig.requestType | ||
| let headers = environment.headers(for: requestType) | ||
| let url = environment.url(for: requestType) |
There was a problem hiding this comment.
Are there any specific client caching configs we should consider here? The server should return a TTL of 10 minutes for the API so I imagine this will be automatically respected here, but just checking.
There was a problem hiding this comment.
I‘d say no as the caching policies should be set on the backend side: if there‘s caching it should be fine as all of our requests are parametrized (i.e. revision, hash etc)
mallexxx
changed the title
Task/Issue URL: https://app.asana.com/0/481882893211075/1208033567421351/f BSK PR: duckduckgo/BrowserServicesKit#1092 **Description**: - Refactor APIClient to use Networking.APIRequestV2; implement generic request/response (BSK) **Optional E2E tests**: - [ ] Run PIR E2E tests Check this to run the Personal Information Removal end to end tests. If updating CCF, or any PIR related code, tick this. **Steps to test this PR**: 1. Validate Phishing Protection change sets and Match API calls work as before <!-- Tagging instructions If this PR isn't ready to be merged for whatever reason it should be marked with the `DO NOT MERGE` label (particularly if it's a draft) If it's pending Product Review/PFR, please add the `Pending Product Review` label. If at any point it isn't actively being worked on/ready for review/otherwise moving forward (besides the above PR/PFR exception) strongly consider closing it (or not opening it in the first place). If you decide not to close it, make sure it's labelled to make it clear the PRs state and comment with more information. --> **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/481882893211075/1208033567421351/f
iOS PR:
macOS PR:
What kind of version bump will this require?: Major
Optional:
Tech Design URL:
CC:
Description:
APIClientto useNetworking.APIRequestV2; implement generic request/responseSteps to test this PR:
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template