Adding Isochrone API support

[langsmith]

This pr resolves #905 by adding support for the Mapbox Isochrone API.

As @zugaldia said in #905 (comment), this isochrone support work is basically the same that was done to add support for the Tilequery API.

The Mapbox Android demo app has an Isochrone API example (pr is here), but the API is interacted with in that example in a more raw way than using a formal Mapbox Java SDK wrapper.

[langsmith]

Would love 👁👁 on this when you get a chance @osana . No rush on my end.

• I get the following error when I run MapboxIsochroneTest on my local machine. It's the same crash that is in CircleCI.

Any idea what I'm doing wrong? I'm not very familiar with AutoValue.

• The code within build() is probably the area that will need most adjusting. Tilequery immediately does autoBuild() inside of its build() method, so that's what I mirrored with isochrone. I don't think MapboxIsochrone would require me to use other methods inside of build() like some of the other Java wrappers do (e.g. https://github.com/mapbox/mapbox-java/blob/master/services-optimization/src/main/java/com/mapbox/api/optimization/v1/MapboxOptimization.java#L489-L493)

[osana]

@langsmith You are missing @AutoValue annotation at the very top of the class

 * @see <a href="https://docs.mapbox.com/api/navigation/#isochrone">API documentation</a>
  * @since 4.5.0
  */
+@AutoValue
 public abstract class MapboxIsochrone extends MapboxService<FeatureCollection, IsochroneService> {

Also isochrone should be added to the TESTABLE_MODULES list in project's build.gradle:

def TESTABLE_MODULES = ["services",
                         "services-core",
                         "services-directions",
                         "services-geocoding",
                         "services-geojson",
+                        "services-isochrone",
                         "services-matching",

[langsmith]

D'oh. Thanks @osana . Just pushed refactoring to address build.gradle and the @AutoValue annotation.

[langsmith]

Running tests on my local machine seems to be working, so now it's a matter of fixing the failing ones 💪 . Progress...

[osana]

💪 Make sure @since tags are properly updated as well

[osana]

@langsmith tags should be 4.6.0 (I believe) as this is a new feature and we are currently at 4.5.0

[langsmith]

@langsmith tags should be 4.6.0 (I believe) as this is a new feature and we are currently at 4.5.0

🤦‍♂️ , right, right.

[langsmith]

fyi, the @since additions in non-isochrone-related classes was because of checkstyle

[osana]

This one does not seem to be used.
It looks like all the tests will be using : ISOCHRONE_WITH_POLYGONS_VALID

[osana]

Another thought that we usually allow a List of objects to be given to a Builder, that in turn creates a String out of a list that is request friendly.

[langsmith]

Another thought that we usually allow a List of objects to be given to a Builder, that in turn creates a String out of a list that is request friendly.

👍 Ok @osana , I just pushed a commit which enables passing through List<Integer>.

• https://github.com/mapbox/mapbox-java/pull/988/files#diff-3b322564527a1e662e970782b2f3f1a2R198
• https://github.com/mapbox/mapbox-java/pull/988/files#diff-3b322564527a1e662e970782b2f3f1a2R291

[langsmith]

Ok @osana . I've adjusted the contour minutes and colors methods to take String ... instead of List<String>. Ready for another round of review when you get a chance 🙇

[osana]

I think this is looking pretty good.

There is one thing would be nice to fix.
You could say that there are two types of tests:

• the ones that test exceptions and request generation
• the ones that use mock fixtures

The mock fixtures ones need to have requests match the request in the fixture. Otherwise, when we do get a failing test, it is misleading to look at the request parameters.