Skip to content

Commit c55e398

Browse files
[google_maps_flutter] Use structured Pigeon data on iOS (#8142)
Converts most of the maps objects that were still using pre-Pigeon JSON serializations to use structured Pigeon classes instead, mirroring the recent work in the Android implementation: - Copies the Pigeon definitions from Android, with minor modifications. - Copies the Dart conversion code and tests from Android, with minor modifications. - Updates the native code to eliminate a lot of JSON boilerplate. In addition to adding type safety, this mostly finishes addressing technical debt dating back to the initial federation of the plugin, where native code in the implementation package is relying on the JSON serialization of objects that is implemented in the platform interface package, meaning that the stringly-typed data had to match *across packages* in addition to languages. This also backports some Dart unit test improvements to the Android implementation. While bringing the test code over I noticed that the expectations were based on running the Pigeon serialization and then asserting things about the results, which I missed in review. That approach is very fragile because the Pigeon serialization is an internal implementation detail of Pigeon and subject to change at any time. Instead the tests should be using the object directly. Part of flutter/flutter#117907
1 parent 913b99e commit c55e398

25 files changed

+3921
-1159
lines changed

packages/google_maps_flutter/google_maps_flutter_android/test/google_maps_flutter_android_test.dart

Lines changed: 86 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -310,43 +310,32 @@ void main() {
310310
// Object two should be changed.
311311
{
312312
expect(toChange.length, 1);
313-
final List<Object?>? encoded = toChange.first.encode() as List<Object?>?;
314-
expect(encoded?.getRange(0, 6), <Object?>[
315-
object2new.consumeTapEvents,
316-
object2new.fillColor.value,
317-
object2new.strokeColor.value,
318-
object2new.visible,
319-
object2new.strokeWidth,
320-
object2new.zIndex.toDouble(),
321-
]);
322-
final PlatformLatLng? latLng = encoded?[6] as PlatformLatLng?;
323-
expect(latLng?.latitude, object2new.center.latitude);
324-
expect(latLng?.longitude, object2new.center.longitude);
325-
expect(encoded?.getRange(7, 9), <Object?>[
326-
object2new.radius,
327-
object2new.circleId.value,
328-
]);
313+
final PlatformCircle firstChanged = toChange.first;
314+
expect(firstChanged.consumeTapEvents, object2new.consumeTapEvents);
315+
expect(firstChanged.fillColor, object2new.fillColor.value);
316+
expect(firstChanged.strokeColor, object2new.strokeColor.value);
317+
expect(firstChanged.visible, object2new.visible);
318+
expect(firstChanged.strokeWidth, object2new.strokeWidth);
319+
expect(firstChanged.zIndex, object2new.zIndex.toDouble());
320+
expect(firstChanged.center.latitude, object2new.center.latitude);
321+
expect(firstChanged.center.longitude, object2new.center.longitude);
322+
expect(firstChanged.radius, object2new.radius);
323+
expect(firstChanged.circleId, object2new.circleId.value);
329324
}
330325
// Object 3 should be added.
331-
expect(toAdd.length, 1);
332326
{
333327
expect(toAdd.length, 1);
334-
final List<Object?>? encoded = toAdd.first.encode() as List<Object?>?;
335-
expect(encoded?.getRange(0, 6), <Object?>[
336-
object3.consumeTapEvents,
337-
object3.fillColor.value,
338-
object3.strokeColor.value,
339-
object3.visible,
340-
object3.strokeWidth,
341-
object3.zIndex.toDouble(),
342-
]);
343-
final PlatformLatLng? latLng = encoded?[6] as PlatformLatLng?;
344-
expect(latLng?.latitude, object3.center.latitude);
345-
expect(latLng?.longitude, object3.center.longitude);
346-
expect(encoded?.getRange(7, 9), <Object?>[
347-
object3.radius,
348-
object3.circleId.value,
349-
]);
328+
final PlatformCircle firstAdded = toAdd.first;
329+
expect(firstAdded.consumeTapEvents, object3.consumeTapEvents);
330+
expect(firstAdded.fillColor, object3.fillColor.value);
331+
expect(firstAdded.strokeColor, object3.strokeColor.value);
332+
expect(firstAdded.visible, object3.visible);
333+
expect(firstAdded.strokeWidth, object3.strokeWidth);
334+
expect(firstAdded.zIndex, object3.zIndex.toDouble());
335+
expect(firstAdded.center.latitude, object3.center.latitude);
336+
expect(firstAdded.center.longitude, object3.center.longitude);
337+
expect(firstAdded.radius, object3.radius);
338+
expect(firstAdded.circleId, object3.circleId.value);
350339
}
351340
});
352341

@@ -406,72 +395,58 @@ void main() {
406395
// Object two should be changed.
407396
{
408397
expect(toChange.length, 1);
409-
final List<Object?>? encoded = toChange.first.encode() as List<Object?>?;
410-
expect(encoded?[0], object2new.alpha);
411-
final PlatformDoublePair? offset = encoded?[1] as PlatformDoublePair?;
412-
expect(offset?.x, object2new.anchor.dx);
413-
expect(offset?.y, object2new.anchor.dy);
414-
expect(encoded?.getRange(2, 5).toList(), <Object?>[
415-
object2new.consumeTapEvents,
416-
object2new.draggable,
417-
object2new.flat,
418-
]);
398+
final PlatformMarker firstChanged = toChange.first;
399+
expect(firstChanged.alpha, object2new.alpha);
400+
expect(firstChanged.anchor.x, object2new.anchor.dx);
401+
expect(firstChanged.anchor.y, object2new.anchor.dy);
402+
expect(firstChanged.consumeTapEvents, object2new.consumeTapEvents);
403+
expect(firstChanged.draggable, object2new.draggable);
404+
expect(firstChanged.flat, object2new.flat);
419405
expect(
420-
(encoded?[5] as PlatformBitmap?)?.bitmap.runtimeType,
406+
firstChanged.icon.bitmap.runtimeType,
421407
GoogleMapsFlutterAndroid.platformBitmapFromBitmapDescriptor(
422408
object2new.icon)
423409
.bitmap
424410
.runtimeType);
425-
final PlatformInfoWindow? window = encoded?[6] as PlatformInfoWindow?;
426-
expect(window?.title, object2new.infoWindow.title);
427-
expect(window?.snippet, object2new.infoWindow.snippet);
428-
expect(window?.anchor.x, object2new.infoWindow.anchor.dx);
429-
expect(window?.anchor.y, object2new.infoWindow.anchor.dy);
430-
final PlatformLatLng? latLng = encoded?[7] as PlatformLatLng?;
431-
expect(latLng?.latitude, object2new.position.latitude);
432-
expect(latLng?.longitude, object2new.position.longitude);
433-
expect(encoded?.getRange(8, 13), <Object?>[
434-
object2new.rotation,
435-
object2new.visible,
436-
object2new.zIndex,
437-
object2new.markerId.value,
438-
object2new.clusterManagerId?.value,
439-
]);
411+
expect(firstChanged.infoWindow.title, object2new.infoWindow.title);
412+
expect(firstChanged.infoWindow.snippet, object2new.infoWindow.snippet);
413+
expect(firstChanged.infoWindow.anchor.x, object2new.infoWindow.anchor.dx);
414+
expect(firstChanged.infoWindow.anchor.y, object2new.infoWindow.anchor.dy);
415+
expect(firstChanged.position.latitude, object2new.position.latitude);
416+
expect(firstChanged.position.longitude, object2new.position.longitude);
417+
expect(firstChanged.rotation, object2new.rotation);
418+
expect(firstChanged.visible, object2new.visible);
419+
expect(firstChanged.zIndex, object2new.zIndex);
420+
expect(firstChanged.markerId, object2new.markerId.value);
421+
expect(firstChanged.clusterManagerId, object2new.clusterManagerId?.value);
440422
}
441423
// Object 3 should be added.
442424
{
443425
expect(toAdd.length, 1);
444-
final List<Object?>? encoded = toAdd.first.encode() as List<Object?>?;
445-
expect(encoded?[0], object3.alpha);
446-
final PlatformDoublePair? offset = encoded?[1] as PlatformDoublePair?;
447-
expect(offset?.x, object3.anchor.dx);
448-
expect(offset?.y, object3.anchor.dy);
449-
expect(encoded?.getRange(2, 5).toList(), <Object?>[
450-
object3.consumeTapEvents,
451-
object3.draggable,
452-
object3.flat,
453-
]);
426+
final PlatformMarker firstAdded = toAdd.first;
427+
expect(firstAdded.alpha, object3.alpha);
428+
expect(firstAdded.anchor.x, object3.anchor.dx);
429+
expect(firstAdded.anchor.y, object3.anchor.dy);
430+
expect(firstAdded.consumeTapEvents, object3.consumeTapEvents);
431+
expect(firstAdded.draggable, object3.draggable);
432+
expect(firstAdded.flat, object3.flat);
454433
expect(
455-
(encoded?[5] as PlatformBitmap?)?.bitmap.runtimeType,
434+
firstAdded.icon.bitmap.runtimeType,
456435
GoogleMapsFlutterAndroid.platformBitmapFromBitmapDescriptor(
457436
object3.icon)
458437
.bitmap
459438
.runtimeType);
460-
final PlatformInfoWindow? window = encoded?[6] as PlatformInfoWindow?;
461-
expect(window?.title, object3.infoWindow.title);
462-
expect(window?.snippet, object3.infoWindow.snippet);
463-
expect(window?.anchor.x, object3.infoWindow.anchor.dx);
464-
expect(window?.anchor.y, object3.infoWindow.anchor.dy);
465-
final PlatformLatLng? latLng = encoded?[7] as PlatformLatLng?;
466-
expect(latLng?.latitude, object3.position.latitude);
467-
expect(latLng?.longitude, object3.position.longitude);
468-
expect(encoded?.getRange(8, 13), <Object?>[
469-
object3.rotation,
470-
object3.visible,
471-
object3.zIndex,
472-
object3.markerId.value,
473-
object3.clusterManagerId?.value,
474-
]);
439+
expect(firstAdded.infoWindow.title, object3.infoWindow.title);
440+
expect(firstAdded.infoWindow.snippet, object3.infoWindow.snippet);
441+
expect(firstAdded.infoWindow.anchor.x, object3.infoWindow.anchor.dx);
442+
expect(firstAdded.infoWindow.anchor.y, object3.infoWindow.anchor.dy);
443+
expect(firstAdded.position.latitude, object3.position.latitude);
444+
expect(firstAdded.position.longitude, object3.position.longitude);
445+
expect(firstAdded.rotation, object3.rotation);
446+
expect(firstAdded.visible, object3.visible);
447+
expect(firstAdded.zIndex, object3.zIndex);
448+
expect(firstAdded.markerId, object3.markerId.value);
449+
expect(firstAdded.clusterManagerId, object3.clusterManagerId?.value);
475450
}
476451
});
477452

@@ -500,17 +475,14 @@ void main() {
500475
expect(toRemove.length, 1);
501476
expect(toRemove.first, object1.polygonId.value);
502477
void expectPolygon(PlatformPolygon actual, Polygon expected) {
503-
final List<Object?> encoded = actual.encode() as List<Object?>;
504-
expect(encoded.sublist(0, 4), <Object>[
505-
expected.polygonId.value,
506-
expected.consumeTapEvents,
507-
expected.fillColor.value,
508-
expected.geodesic,
509-
]);
478+
expect(actual.polygonId, expected.polygonId.value);
479+
expect(actual.consumesTapEvents, expected.consumeTapEvents);
480+
expect(actual.fillColor, expected.fillColor.value);
481+
expect(actual.geodesic, expected.geodesic);
510482
expect(actual.points.length, expected.points.length);
511483
for (final (int i, PlatformLatLng? point) in actual.points.indexed) {
512-
expect(point?.latitude, actual.points[i].latitude);
513-
expect(point?.longitude, actual.points[i].longitude);
484+
expect(point?.latitude, expected.points[i].latitude);
485+
expect(point?.longitude, expected.points[i].longitude);
514486
}
515487
expect(actual.holes.length, expected.holes.length);
516488
for (final (int i, List<PlatformLatLng>? hole) in actual.holes.indexed) {
@@ -520,12 +492,10 @@ void main() {
520492
expect(point?.longitude, expectedHole[j].longitude);
521493
}
522494
}
523-
expect(encoded.sublist(6), <Object>[
524-
expected.visible,
525-
expected.strokeColor.value,
526-
expected.strokeWidth,
527-
expected.zIndex,
528-
]);
495+
expect(actual.visible, expected.visible);
496+
expect(actual.strokeColor, expected.strokeColor.value);
497+
expect(actual.strokeWidth, expected.strokeWidth);
498+
expect(actual.zIndex, expected.zIndex);
529499
}
530500

531501
// Object two should be changed.
@@ -564,19 +534,15 @@ void main() {
564534
verification.captured[1] as List<PlatformPolyline>;
565535
final List<String> toRemove = verification.captured[2] as List<String>;
566536
void expectPolyline(PlatformPolyline actual, Polyline expected) {
567-
final List<Object?> encoded = actual.encode() as List<Object?>;
568-
expect(encoded.sublist(0, 5), <Object?>[
569-
expected.polylineId.value,
570-
expected.consumeTapEvents,
571-
expected.color.value,
572-
expected.geodesic,
573-
platformJointTypeFromJointType(expected.jointType),
574-
]);
575-
expect(encoded.sublist(9), <Object?>[
576-
expected.visible,
577-
expected.width,
578-
expected.zIndex,
579-
]);
537+
expect(actual.polylineId, expected.polylineId.value);
538+
expect(actual.consumesTapEvents, expected.consumeTapEvents);
539+
expect(actual.color, expected.color.value);
540+
expect(actual.geodesic, expected.geodesic);
541+
expect(
542+
actual.jointType, platformJointTypeFromJointType(expected.jointType));
543+
expect(actual.visible, expected.visible);
544+
expect(actual.width, expected.width);
545+
expect(actual.zIndex, expected.zIndex);
580546
expect(actual.points.length, expected.points.length);
581547
for (final (int i, PlatformLatLng? point) in actual.points.indexed) {
582548
expect(point?.latitude, actual.points[i].latitude);
@@ -639,15 +605,13 @@ void main() {
639605
final List<PlatformTileOverlay> toChange =
640606
verification.captured[1] as List<PlatformTileOverlay>;
641607
final List<String> toRemove = verification.captured[2] as List<String>;
642-
void expectTileOverlay(PlatformTileOverlay? actual, TileOverlay expected) {
643-
expect(actual?.encode(), <Object?>[
644-
expected.tileOverlayId.value,
645-
expected.fadeIn,
646-
expected.transparency,
647-
expected.zIndex,
648-
expected.visible,
649-
expected.tileSize,
650-
]);
608+
void expectTileOverlay(PlatformTileOverlay actual, TileOverlay expected) {
609+
expect(actual.tileOverlayId, expected.tileOverlayId.value);
610+
expect(actual.fadeIn, expected.fadeIn);
611+
expect(actual.transparency, expected.transparency);
612+
expect(actual.zIndex, expected.zIndex);
613+
expect(actual.visible, expected.visible);
614+
expect(actual.tileSize, expected.tileSize);
651615
}
652616

653617
// Object one should be removed.

packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 2.13.2
2+
3+
* Updates most objects passed from Dart to native to use typed data.
4+
15
## 2.13.1
26

37
* Updates Pigeon for non-nullable collection type support.

0 commit comments

Comments
 (0)