Skip to content

Conversation

@be-neth
Copy link
Contributor

@be-neth be-neth commented Jan 28, 2017

Refactor of pull request #7 .

User can choose between OSM or MapQuest API server.
Use HTTPS.

OSM API server

Free, without API key.
For usage policy see : http://wiki.openstreetmap.org/wiki/Nominatim_usage_policy

MapQuest API server

Needs to have an API key.
Limited to : 15,000 transactions/month
See : https://developer.mapquest.com/

be-neth and others added 6 commits January 27, 2017 14:07
MapQuest now requires a developer API key to be used when querying their
Nominatim server. This change allows the end user to enter a key that
they can get without charge from the MapQuest developer site at

https://developer.mapquest.com

Signed-off-by: Tod Fitch <Tod@FitchDesign.com>
Android Studio was too helpful in suggesting changes and automatically editing
the manifest.

Signed-off-by: Tod Fitch <Tod@FitchDesign.com>
@be-neth be-neth changed the title N76/issue4/refactor : N76/issue4/refactor : Choose API server Jan 28, 2017
@be-neth be-neth force-pushed the n76/issue4/refactor branch from 7edc021 to f6ca893 Compare January 28, 2017 13:56
@be-neth
Copy link
Contributor Author

be-neth commented Jan 28, 2017

@ale5000-git
Copy link
Member

ale5000-git commented Jan 28, 2017

@be-neth: Can you please leave minSdkVersion to 9?

@be-neth
Copy link
Contributor Author

be-neth commented Jan 28, 2017

@ale5000-git No, sorry.

I answered you on the other pull request comment:

@ale5000-git It is not possible because it uses getFragmentManager() which is available since API 11.
https://developer.android.com/reference/android/app/FragmentManager.html

@mar-v-in
Copy link
Member

You can use the v4 support library to still keep minSdkVersion at 9.

try {
Log.d(TAG, "Requesting " + url);
HttpURLConnection connection = (HttpURLConnection) new URL(url)
HttpsURLConnection connection = (HttpsURLConnection) new URL(url)
Copy link
Member

Choose a reason for hiding this comment

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

You can keep this at HttpURLConnection as you don't use any feature of HttpsURLConnection. Casting this to a HttpsURLConnection instance does not improve security ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Casting this to a HttpsURLConnection instance does not improve security ;)

Yes obviously :)
But maybe it can be a good practice to use HttpsURLConnection object in case we need in the futur to play with the HTTPS connection parameters.

Copy link
Member

Choose a reason for hiding this comment

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

I tend to use the least specific class that still suites my needs, in this case that would be HttpURLConnection. Just consider that a future Java version might decide that http 2.0 is not done using HttpsURLConnection, although it es encrypted. It's also very unlikely that we ever want to fiddle with tls parameters and finally using classes from the javax package should generally be done only if really needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.
Revert done.

@be-neth
Copy link
Contributor Author

be-neth commented Jan 29, 2017

You can use the v4 support library to still keep minSdkVersion at 9.

In the end I use PreferenceFragmentCompat from android v7 support library.

The only difference is I cannot subclass PreferenceActivity anymore but I need to use AppCompatActivity to have : getSupportFragmentManager().

Thanks @mar-v-in for the reference.

@mar-v-in mar-v-in merged commit 00d5a41 into microg:master Jan 29, 2017
@mar-v-in
Copy link
Member

mar-v-in commented Jan 29, 2017

Great work! Includes @n76 of course ;)

@mar-v-in mar-v-in mentioned this pull request Jan 29, 2017
This was referenced Jan 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants