Include concrete device model in user agent (PSG-754) #1576
Conversation
Johennes
requested review from
a team and
Anderas
and removed request for
a team
| @@ -649,6 +651,27 @@ - (void)setRequestParametersInJSON:(BOOL)requestParametersInJSON | |||
| { | |||
| httpManager.requestSerializer = [AFHTTPRequestSerializer serializer]; | |||
| } | |||
|
|
|||
| // Overwrite user agent to include concrete device model (code copied from AlamoFire) | |||
There was a problem hiding this comment.
The code below is copied from AFHTTPRequestSerializer. I decided to keep this in the ObjC part because this way it's an exact copy and possibly easier to maintain than a Swift port.
| @@ -14,6 +14,7 @@ PODS: | |||
| - AFNetworking/Serialization (4.0.1) | |||
| - AFNetworking/UIKit (4.0.1): | |||
| - AFNetworking/NSURLSession | |||
| - DeviceKit (4.7.0) | |||
There was a problem hiding this comment.
This is the best option I found for converting Apple's device identifiers into human readable strings. It's MIT, relatively small, well-maintained and somewhat popular so I hope this is an ok choice.
Johennes
force-pushed
the
johannes/concrete-device-model-user-agent
branch
from
c928c60 to
9695758
Compare
Codecov ReportBase: 12.49% // Head: 36.98% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1576 +/- ##
============================================
+ Coverage 12.49% 36.98% +24.48%
============================================
Files 521 522 +1
Lines 85000 85019 +19
Branches 36193 37544 +1351
============================================
+ Hits 10624 31441 +20817
+ Misses 73990 52614 -21376
- Partials 386 964 +578
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Johennes
requested review from
a team and
ismailgulek
and removed request for
Anderas and
a team
|
Rolling the reviewer assignment dice again because I think Andy is busy with the releases. |
There was a problem hiding this comment.
Changes LGTM but I'd like to check with @manuroe that should this change be at the sdk or at the app side (can be done via MXSDKOptions.additionalHTTPHeaders)?
Good point. If possible, we would prefer this customisation only at the app level for apps that require it. |
|
Sounds good. Closing this in favor of element-hq/element-ios#6761. |
Fixes element-hq/element-ios#6742
Pull Request Checklist