Deprecated serialized fields

[VysotskiVadim]

mapbox-java used to serialized access-token and uuid when a user calls RouteOptions#toJson(). You will get an invalid URL If you deserialize RouteOptions from a json, which contains access-token or uuid, and then call RouteOptions#toURL.

[VysotskiVadim]

@Guardiola31337 , @LukasPaczos , @mapbox/navigation-android , do you think this simple mechanism is sufficient?

[LukasPaczos]

LGTM

[LukasPaczos]

A warning log could be good.

[codecov]

Codecov Report

Merging #1447 (3ee83bc) into main (cb85935) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #1447      +/-   ##
============================================
+ Coverage     75.63%   75.67%   +0.04%     
- Complexity      879      881       +2     
============================================
  Files           125      125              
  Lines          3899     3906       +7     
  Branches        577      578       +1     
============================================
+ Hits           2949     2956       +7     
  Misses          687      687              
  Partials        263      263              

Impacted Files	Coverage Δ
.../mapbox/api/directions/v5/models/RouteOptions.java	85.86% <100.00%> (+0.55%)	⬆️

[RingerJK]

LGTM. Let's add a check (+logs) to toUrl that checks that we don't have the same queries twice or more time. wdyt?

[VysotskiVadim]

Let's add a check (+logs) to toUrl that checks that we don't have the same queries twice or more time. wdyt?

@RingerJK , how do you think, what's the best think we can do if there're duplicated parameters? Throw an exceptions with an understandable description?

[RingerJK]

I think logs with w level should be good.

[Guardiola31337]

Should we add a sanity test that serializes a legacy DirectionsResponse JSON like multiple_routes.json and verifies that the URL doesn't contain neither a duplicated access_token nor the uuid? 🤔

[VysotskiVadim]

Should we add a sanity test that serializes a legacy DirectionsResponse JSON like multiple_routes.json and verifies that the URL doesn't contain neither a duplicated access_token nor the uuid? 🤔

added, see sanityCheckForPreviouslySerializedRouteResponse

[VysotskiVadim]

I think logs with w level should be good.

@RingerJK , @LukasPaczos , I don't see any logging mechanisms in mapbox-java. Do you think it's time to introduce one?

TBH, I don't think that it's time 🙂

[RingerJK]

I think logs with w level should be good.

@RingerJK , @LukasPaczos , I don't see any logging mechanisms in mapbox-java. Do you think it's time to introduce one?

TBH, I don't think that it's time 🙂

suppose, mapbox-java contains business logic, so it's time to investigate if it needs a logging mechanism. Let's cut a ticket

[VysotskiVadim]

I think logs with w level should be good.

@RingerJK , @LukasPaczos , I don't see any logging mechanisms in mapbox-java. Do you think it's time to introduce one?
TBH, I don't think that it's time 🙂

suppose, mapbox-java contains business logic, so it's time to investigate if it needs a logging mechanism. Let's cut a ticket

#1448

[RingerJK]

as we still don't have logger, could you put // TODO log.W("CONTEXT") (replace W by any other level where it needs) to places that must be logged

[VysotskiVadim]

as we still don't have logger, could you put // TODO log.W("CONTEXT") (replace W by any other level where it needs) to places that must be logged

Added to the logger ticket instead #1448 (comment)

[RingerJK]

LGTM