diff --git a/src/SDK/Language/Flutter.php b/src/SDK/Language/Flutter.php index a7ccea44a3..ede171775f 100644 --- a/src/SDK/Language/Flutter.php +++ b/src/SDK/Language/Flutter.php @@ -324,11 +324,13 @@ public function getFiles(): array [ 'scope' => 'default', 'destination' => '/lib/src/analytics_observer.dart', + 'requires' => 'analytics', 'template' => 'flutter/lib/src/analytics_observer.dart.twig', ], [ 'scope' => 'default', 'destination' => '/lib/src/analytics_tracking.dart', + 'requires' => 'analytics', 'template' => 'flutter/lib/src/analytics_tracking.dart.twig', ], [ diff --git a/src/SDK/Language/Web.php b/src/SDK/Language/Web.php index db8a921a24..9072f30fa8 100644 --- a/src/SDK/Language/Web.php +++ b/src/SDK/Language/Web.php @@ -66,6 +66,7 @@ public function getFiles(): array [ 'scope' => 'default', 'destination' => 'src/services/analytics-tracking.ts', + 'requires' => 'analytics', 'template' => 'web/src/services/analytics-tracking.ts.twig', ], [ diff --git a/src/SDK/SDK.php b/src/SDK/SDK.php index dd1480d0bc..9674c70e4f 100644 --- a/src/SDK/SDK.php +++ b/src/SDK/SDK.php @@ -1012,6 +1012,13 @@ public function generate(string $target): void continue; } + // A companion template that references a generated service must not + // ship when the spec carries no such service, or it names a class + // the SDK never generates and the package stops compiling. + if (isset($file['requires']) && !isset($filteredServices[$file['requires']])) { + continue; + } + if (!\in_array($file['scope'], ['copy', 'download'], true)) { $template = $this->twig->load($file['template']); /* @var $template TemplateWrapper */ } diff --git a/templates/flutter/lib/package.dart.twig b/templates/flutter/lib/package.dart.twig index 6fe340b2f2..f54d71f869 100644 --- a/templates/flutter/lib/package.dart.twig +++ b/templates/flutter/lib/package.dart.twig @@ -30,8 +30,10 @@ export 'src/upload_progress.dart'; export 'src/realtime_subscription.dart'; export 'src/realtime_message.dart'; export 'src/input_file.dart'; +{% if services['analytics'] is defined %} export 'src/analytics_observer.dart'; export 'src/analytics_tracking.dart'; +{% endif %} part 'query.dart'; part 'permission.dart'; diff --git a/templates/flutter/lib/src/analytics_observer.dart.twig b/templates/flutter/lib/src/analytics_observer.dart.twig index ff9ec85a64..d79d0bac59 100644 --- a/templates/flutter/lib/src/analytics_observer.dart.twig +++ b/templates/flutter/lib/src/analytics_observer.dart.twig @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/widgets.dart'; /// Signature used to emit an analytics event from an [AnalyticsObserver] or @@ -27,7 +29,7 @@ import 'package:flutter/widgets.dart'; /// [propertyId] and [engagementTime] map to the endpoint's top-level params of /// the same name; both are null unless the helper has something to say about /// them, so a closure is free to ignore either. -typedef AnalyticsEventEmitter = void Function( +typedef AnalyticsEventEmitter = FutureOr Function( String name, { Map? props, String? propertyId, @@ -96,20 +98,33 @@ class AnalyticsObserver extends NavigatorObserver { this.propertyId, }) : nameExtractor = nameExtractor ?? defaultScreenNameExtractor; + /// The route the user is actually looking at. `didRemove` fires for buried + /// routes too, and its `previousRoute` is the route below the removed one + /// rather than the visible one, so the top has to be tracked explicitly. + Route? _topRoute; + @override void didPush(Route route, Route? previousRoute) { + _topRoute = route; _sendScreenView(route, previousRoute, trigger: 'push'); } @override void didReplace({Route? newRoute, Route? oldRoute}) { - if (newRoute != null) { - _sendScreenView(newRoute, oldRoute, trigger: 'replace'); + if (newRoute == null) { + return; } + if (oldRoute != null && !identical(oldRoute, _topRoute)) { + // A replace further down the stack leaves the visible screen alone. + return; + } + _topRoute = newRoute; + _sendScreenView(newRoute, oldRoute, trigger: 'replace'); } @override void didPop(Route route, Route? previousRoute) { + _topRoute = previousRoute; if (previousRoute != null) { _sendScreenView(previousRoute, route, trigger: 'pop'); } @@ -117,6 +132,11 @@ class AnalyticsObserver extends NavigatorObserver { @override void didRemove(Route route, Route? previousRoute) { + if (!identical(route, _topRoute)) { + // Removing a buried route never changes what is on screen. + return; + } + _topRoute = previousRoute; if (previousRoute != null) { _sendScreenView(previousRoute, route, trigger: 'remove'); } @@ -140,7 +160,14 @@ class AnalyticsObserver extends NavigatorObserver { props['previous'] = previousName; } try { - emit(eventName, props: props, propertyId: propertyId); + // The emitter usually returns the request future; without catching it a + // rejected request surfaces as an unhandled async error in the host app. + final pending = emit(eventName, props: props, propertyId: propertyId); + if (pending is Future) { + // `catchError` on a typed future must return that future's type, so an + // empty handler would itself throw. Narrow to `Future` first. + unawaited(pending.then((_) {}).catchError((Object _) {})); + } } catch (_) { // Emitter failures must never propagate into the navigator stack — // swallow so analytics can't crash the app. diff --git a/templates/flutter/lib/src/analytics_tracking.dart.twig b/templates/flutter/lib/src/analytics_tracking.dart.twig index 760632a088..0026e984a6 100644 --- a/templates/flutter/lib/src/analytics_tracking.dart.twig +++ b/templates/flutter/lib/src/analytics_tracking.dart.twig @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/widgets.dart'; import 'analytics_observer.dart'; @@ -123,12 +125,19 @@ class AnalyticsTracking with WidgetsBindingObserver { int? engagementTime, }) { try { - _emit( + // The emitter usually returns the request future; without catching it a + // rejected request surfaces as an unhandled async error in the host app. + final pending = _emit( name, props: props, propertyId: propertyId, engagementTime: engagementTime, ); + if (pending is Future) { + // `catchError` on a typed future must return that future's type, so an + // empty handler would itself throw. Narrow to `Future` first. + unawaited(pending.then((_) {}).catchError((Object _) {})); + } } catch (_) { // Silent — see class docs. } @@ -177,6 +186,13 @@ class AnalyticsTracking with WidgetsBindingObserver { ); break; case AppLifecycleState.resumed: + // `inactive` -> `resumed` (notification shade, app switcher preview) + // never reaches `hidden`/`paused`, so the interval is still open. + // Restarting it would discard the engagement accrued before the + // interruption and emit a foreground event for no background. + if (_foregroundSince != null) { + break; + } _foregroundSince = DateTime.now(); event('app_foregrounded'); break; diff --git a/templates/web/src/index.ts.twig b/templates/web/src/index.ts.twig index fb521a266c..e26d7ccf29 100644 --- a/templates/web/src/index.ts.twig +++ b/templates/web/src/index.ts.twig @@ -13,7 +13,9 @@ export { {{service.name | caseUcfirst}} } from './services/{{service.name | case {% endfor %} export { Realtime } from './services/realtime'; export { Push } from './services/push'; +{% if services['analytics'] is defined %} export { AnalyticsTracking } from './services/analytics-tracking'; +{% endif %} export type { Models, Payload, @@ -27,6 +29,7 @@ export type { SubscribeOptions, MessageCallback, } from './services/push'; +{% if services['analytics'] is defined %} export type { AnalyticsEventEmitter, AnalyticsEventOptions, @@ -34,6 +37,7 @@ export type { DownloadTrackingOptions, OutboundTrackingOptions, } from './services/analytics-tracking'; +{% endif %} export type { QueryTypes, QueryTypesList } from './query'; export { Permission } from './permission'; export { Role } from './role'; diff --git a/tests/generation/GenerationTest.php b/tests/generation/GenerationTest.php index 8330538e8d..db8dcb0bc8 100644 --- a/tests/generation/GenerationTest.php +++ b/tests/generation/GenerationTest.php @@ -293,6 +293,76 @@ public function testFixtureSelection(string $name): void } } + /** + * The analytics tracking helpers are hand-written companions to a generated + * service. Shipping them into an SDK whose spec has no analytics service + * leaves files, and entry-point exports, naming a class that is never + * generated — which is what broke appwrite/sdk-for-flutter#336. + */ + public function testAnalyticsCompanionsFollowTheService(): void + { + $companions = [ + 'flutter' => ['lib/src/analytics_observer.dart', 'lib/src/analytics_tracking.dart'], + 'web' => ['src/services/analytics-tracking.ts'], + ]; + $entrypoints = [ + 'flutter' => ['lib/packageName.dart', 'analytics_observer.dart'], + 'web' => ['src/index.ts', 'analytics-tracking'], + ]; + + foreach ($companions as $name => $paths) { + // The shared fixture carries no analytics service. + $without = $this->generate($name, 'client'); + foreach ($paths as $path) { + $this->assertArrayNotHasKey($path, $without, "{$name} shipped {$path} without the analytics service"); + } + [$entry, $token] = $entrypoints[$name]; + $this->assertArrayHasKey($entry, $without, "{$name} did not generate {$entry}"); + $this->assertStringNotContainsString($token, $without[$entry], "{$name} {$entry} exports a companion it did not generate"); + + $with = $this->generateWithAnalytics($name); + foreach ($paths as $path) { + $this->assertArrayHasKey($path, $with, "{$name} did not ship {$path} alongside the analytics service"); + } + $this->assertStringContainsString($token, $with[$entry], "{$name} {$entry} does not export the companion"); + } + } + + /** + * @return array + */ + private function generateWithAnalytics(string $name): array + { + $dir = self::OUTPUT . '/' . $name . '/analytics'; + $this->removeDirectory($dir); + + $document = \json_decode((string) \file_get_contents(self::FIXTURE), flags: JSON_THROW_ON_ERROR); + $operation = clone $document->paths->{'/ping'}->get; + $operation->tags = ['analytics']; + $operation->operationId = 'analyticsCreateEvent'; + $document->paths->{'/analytics/event'} = (object) ['get' => $operation]; + $document->tags[] = (object) ['name' => 'analytics', 'description' => 'Analytics']; + + $sdk = new SDK($this->language($name), Parser::parse(\json_encode($document, JSON_THROW_ON_ERROR))); + $sdk + ->setName('test') + ->setVersion('0.0.1') + ->setPlatform('client') + ->setNamespace('appwrite') + ->setTest('true') + ->generate($dir); + + $files = []; + $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS)); + foreach ($iterator as $file) { + if ($file->isFile()) { + $files[\substr((string) $file->getPathname(), \strlen($dir) + 1)] = \strtolower((string) \file_get_contents($file->getPathname())); + } + } + + return $files; + } + public function testJavascriptClientOwnership(): void { $targets = [