Make DirectionsWaypoint#name @Nullable as @NonNull breaks backwards compatibility

[Guardiola31337]

This PR makes DirectionsWaypoint#name @Nullable as @NonNull breaks backwards compatibility.

Regression from #1360 as older DirectionsResponses may not include the name property which causes DirectionsResponse.fromJson to crash 👀

2022-03-25 14:52:36.467 4305-4305/com.mapbox.navigation.examples E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.mapbox.navigation.examples, PID: 4305
    java.lang.NullPointerException: Null distance
        at com.mapbox.api.directions.v5.models.$AutoValue_DirectionsRoute.<init>($AutoValue_DirectionsRoute.java:40)
        at com.mapbox.api.directions.v5.models.AutoValue_DirectionsRoute.<init>(AutoValue_DirectionsRoute.java:28)
        at com.mapbox.api.directions.v5.models.AutoValue_DirectionsRoute$GsonTypeAdapter.read(AutoValue_DirectionsRoute.java:280)
        at com.mapbox.api.directions.v5.models.AutoValue_DirectionsRoute$GsonTypeAdapter.read(AutoValue_DirectionsRoute.java:31)
        at com.google.gson.Gson.fromJson(Gson.java:932)
        at com.google.gson.Gson.fromJson(Gson.java:1003)
        at com.google.gson.Gson.fromJson(Gson.java:975)
        at com.mapbox.api.directions.v5.models.DirectionsRoute.fromJson(DirectionsRoute.java:182)
        at com.mapbox.navigation.core.history.model.HistoryEventMapper.mapNavigationRoute(HistoryEventMapper.kt:98)
        at com.mapbox.navigation.core.history.model.HistoryEventMapper.mapSetRoute(HistoryEventMapper.kt:63)
        at com.mapbox.navigation.core.history.model.HistoryEventMapper.map(HistoryEventMapper.kt:31)
        at com.mapbox.navigation.core.history.MapboxHistoryReader.loadNext(MapboxHistoryReader.kt:44)
        at com.mapbox.navigation.core.history.MapboxHistoryReader.next(MapboxHistoryReader.kt:38)
        at com.mapbox.navigation.core.history.MapboxHistoryReader.next(MapboxHistoryReader.kt:13)
        at kotlin.sequences.TransformingSequence$iterator$1.next(Sequences.kt:210)
        at kotlin.sequences.FilteringSequence$iterator$1.calcNext(Sequences.kt:170)
        at kotlin.sequences.FilteringSequence$iterator$1.hasNext(Sequences.kt:194)
        at kotlin.sequences.SequencesKt___SequencesKt.toCollection(_Sequences.kt:786)
        at kotlin.sequences.SequencesKt___SequencesKt.toMutableList(_Sequences.kt:816)
        at kotlin.sequences.SequencesKt___SequencesKt.toList(_Sequences.kt:807)
        at com.mapbox.navigation.examples.core.replay.HistoryFileLoader$loadSelectedHistory$2.invokeSuspend(HistoryFileLoader.kt:31)
        at com.mapbox.navigation.examples.core.replay.HistoryFileLoader$loadSelectedHistory$2.invoke(Unknown Source:8)
        at com.mapbox.navigation.examples.core.replay.HistoryFileLoader$loadSelectedHistory$2.invoke(Unknown Source:4)
        at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:89)
        at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:165)
        at kotlinx.coroutines.BuildersKt.withContext(Unknown Source:1)
        at com.mapbox.navigation.examples.core.replay.HistoryFileLoader.loadSelectedHistory(HistoryFileLoader.kt:28)
        at com.mapbox.navigation.examples.core.replay.HistoryFileLoader.access$loadSelectedHistory(HistoryFileLoader.kt:12)
        at com.mapbox.navigation.examples.core.replay.HistoryFileLoader$loadReplayHistory$2.invokeSuspend(HistoryFileLoader.kt:24)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)

[LukasPaczos]

@Guardiola31337 do you know how was that old response created? Directions API documents waypoint name as non-optional.

[Guardiola31337]

For reference, requestUuid was QUaEPAFGYU_CWKMme8Tuqvlfq_VpCr6UvfDOUDtRu31m2av6ABk0zw\\u003d\\u003d, endpoints_config and NN version 👇

"endpoints_config": {
      "host": "https://api.mapbox.com",
      "dataset": "mapbox/driving-traffic",
      "version": "2021_02_14-03_00_00",
      "navigator_version": "79.0.3",
      "userAgent": "MapboxNavigationNative"
    }

and Nav SDK version 2.1.0.

Sharing the route internally as I'm hitting comment is too long (maximum is 65536 characters) error 😅

cc @mapbox/navnative @mapbox/navigation-api @browndp08 @danpat

[LukasPaczos]

For now (until we learn more), let's revert this since it was only part of the v6.4 pre-releases. We can always re-adjust again in the future.

[LukasPaczos]

Could you update the builder method annotation as well?

[Guardiola31337]

Yeah, great catch!

[Guardiola31337]

Ohh wait, the builder is going to be @NonNull always, isn't it? 🤔

[LukasPaczos]

I mean the parameter nullability:

[at]NonNull
public abstract Builder name([at]Nullable String name);

[Guardiola31337]

Wondering if we want to keep it as non-nullable in the Builder, I mean, normally if you want to set a name is going to be non-null (unless that property could be reset or something around those lines). Is the Builder used automatically behind the scenes when parsing? I believe not, because that'd crash when I tested it. Noting that in this particular case, the property was missing i.e. it wasn't null. If we're going to get null by any chance, yeah, the builder should be @Nullable.

[LukasPaczos]

Is the Builder used automatically behind the scenes when parsing?

It's not, but the builder parameter nullability should always reflect the property nullability.