Set User-Agent string for osmdroid #3254
Conversation
| * Get a User-Agent string that provides the platform details followed by the application ID | ||
| * and application version name: {@code Dalvik/<version> (platform info) org.odk.collect.android/v<version>}. | ||
| * | ||
| * This deviates from the recommended format as described in https://github.com/opendatakit/collect/issues/3253. |
There was a problem hiding this comment.
Not a big deal, but I like putting the app ID first, at least in the long term. Did you keep Dalvik first because you'd prefer to keep it first, or just to minimize changes for now?
There was a problem hiding this comment.
Yes, I do as well. I thought it was nice to have a single place where we generate a User-Agent string. And since changing it for all HTTP requests is more of an enhancement than a fix, I'd rather give a chance for #3253 to get comments and do it after v1.23 is out so it can spend some time in beta. I don't think changing it for osmdroid then will have any repercussions.
|
This looks great! |
|
@lognaturel this pr does not have assigned version. Do we want to have it in v1.23? |
|
Sorry about that. Yes, please. I initially wasn't sure it would be a fix for #3248 but it turns out to be. |
|
Tested with success Verified on Android: 4.2, 4.4, 6.0, 7.0, 8.1 I confirm cases:
@opendatakit-bot unlabel "needs testing" |
Closes #3252
Closes #3248
I have communicated with OSM and they have advised us to change the User-Agent string.
What has been done to verify that this works as intended?
Verified that we follow the osmdroid instructions.
Used the network profiler to look at the request details while loading an osmdroid-rendered map and verified that the User-Agent header is as expected (Dalvik/2.1.0 (Linux; U; Android 5.1.1; SM-T285 Build/LMY47V) org.odk.collect.android/v1.23.0-beta.4-9-g4cb530c69).
Why is this the best possible solution? Were any other approaches considered?
I moved the
getUserAgentStringmethod to the application level so all components that send user agent strings can use a consistent one.I considered also addressing #3253 but since I'm not totally sure it's without risk and it's not really a fix, I figured we can do this for the next release.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
It should not affect users.
It does affect tile servers that we pull from, though, because now they'll be seeing a new User-Agent string. In particular, this will bring back access to OpenStreetMap tiles which violates their terms of service. We should wait until we learn more about #3248 before merging.
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.