Merge pull request #118 from flutter-news-app-full-source-code/enhance-ads-feature

Enhance ads feature

Author: fulleni

.vscode/launch.json

@@ -0,0 +1,25 @@
+{
+    // Use IntelliSense to learn about possible attributes.
+    // Hover to view descriptions of existing attributes.
+    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "flutter-news-app-mobile-client-full-source-code",
+            "request": "launch",
+            "type": "dart"
+        },
+        {
+            "name": "flutter-news-app-mobile-client-full-source-code (profile mode)",
+            "request": "launch",
+            "type": "dart",
+            "flutterMode": "profile"
+        },
+        {
+            "name": "flutter-news-app-mobile-client-full-source-code (release mode)",
+            "request": "launch",
+            "type": "dart",
+            "flutterMode": "release"
+        }
+    ]
+}

lib/account/bloc/account_bloc.dart

@@ -5,8 +5,6 @@ import 'package:bloc/bloc.dart';
 import 'package:core/core.dart';
 import 'package:data_repository/data_repository.dart';
 import 'package:equatable/equatable.dart';
-import 'package:flutter_news_app_mobile_client_full_source_code/app/config/config.dart'
-    as local_config;
 import 'package:logging/logging.dart';
 
 part 'account_event.dart';
@@ -17,11 +15,9 @@ class AccountBloc extends Bloc<AccountEvent, AccountState> {
     required AuthRepository authenticationRepository,
     required DataRepository<UserContentPreferences>
     userContentPreferencesRepository,
-    required local_config.AppEnvironment environment,
     Logger? logger,
   }) : _authenticationRepository = authenticationRepository,
        _userContentPreferencesRepository = userContentPreferencesRepository,
-       _environment = environment,
        _logger = logger ?? Logger('AccountBloc'),
        super(const AccountState()) {
     // Listen to user changes from AuthRepository
@@ -57,7 +53,6 @@ class AccountBloc extends Bloc<AccountEvent, AccountState> {
   final AuthRepository _authenticationRepository;
   final DataRepository<UserContentPreferences>
   _userContentPreferencesRepository;
-  final local_config.AppEnvironment _environment;
   final Logger _logger;
   late StreamSubscription<User?> _userSubscription;
   late StreamSubscription<Type> _userContentPreferencesSubscription;

lib/ads/ad_provider.dart

@@ -71,4 +71,9 @@ abstract class AdProvider {
     required String? adId,
     required AdThemeStyle adThemeStyle,
   });
+
+  /// This method is responsible for releasing the resources held by the
+  /// platform-specific ad object (e.g., `google_mobile_ads.Ad`).
+  /// It should be called when an ad is no longer needed to prevent memory leaks.
+  Future<void> disposeAd(Object adObject);
 }

lib/ads/ad_service.dart

@@ -3,6 +3,7 @@ import 'package:flutter_news_app_mobile_client_full_source_code/ads/ad_provider.
 import 'package:flutter_news_app_mobile_client_full_source_code/ads/models/ad_theme_style.dart';
 import 'package:flutter_news_app_mobile_client_full_source_code/ads/models/inline_ad.dart';
 import 'package:flutter_news_app_mobile_client_full_source_code/ads/models/interstitial_ad.dart';
+import 'package:flutter_news_app_mobile_client_full_source_code/app/config/app_environment.dart';
 import 'package:logging/logging.dart';
 
 /// {@template ad_service}
@@ -20,13 +21,21 @@ class AdService {
   /// These providers will be used to load ads from specific ad networks.
   AdService({
     required Map<AdPlatformType, AdProvider> adProviders,
+    required AppEnvironment environment,
     Logger? logger,
   }) : _adProviders = adProviders,
+       _environment = environment,
        _logger = logger ?? Logger('AdService');
 
   final Map<AdPlatformType, AdProvider> _adProviders;
+  final AppEnvironment _environment;
   final Logger _logger;
 
+  // Configurable retry parameters for ad loading.
+  // TODO(fulleni): Make this configurable through the remote config.
+  static const int _maxAdLoadRetries = 2;
+  static const Duration _adLoadRetryDelay = Duration(seconds: 1);
+
   /// Initializes all underlying ad providers.
   ///
   /// This should be called once at application startup to ensure all
@@ -39,6 +48,41 @@ class AdService {
     _logger.info('AdService: AdService initialized.');
   }
 
+  /// Disposes of an ad object by delegating to the appropriate [AdProvider].
+  ///
+  /// This method is called by the [InlineAdCacheService] to ensure that
+  /// inline ad resources are released when an ad is removed from the cache
+  /// or replaced. It also handles disposal of interstitial ads.
+  Future<void> disposeAd(dynamic adModel) async {
+    // Determine the AdPlatformType from the adModel if it's an InlineAd or InterstitialAd.
+    AdPlatformType? providerType;
+    Object? adObject; // To hold the actual ad object
+
+    if (adModel is InlineAd) {
+      providerType = adModel.provider;
+      adObject = adModel.adObject;
+    } else if (adModel is InterstitialAd) {
+      providerType = adModel.provider;
+      adObject = adModel.adObject;
+    }
+
+    if (providerType != null && adObject != null) {
+      final adProvider = _adProviders[providerType];
+      if (adProvider != null) {
+        await adProvider.disposeAd(adObject); // Pass the actual ad object
+      } else {
+        _logger.warning(
+          'AdService: No AdProvider found for type $providerType to dispose ad.',
+        );
+      }
+    } else {
+      _logger.warning(
+        'AdService: Cannot determine AdPlatformType or ad object for ad model of type '
+        '${adModel.runtimeType}. Cannot dispose ad.',
+      );
+    }
+  }
+
   /// Retrieves a loaded inline ad (native or banner) for display in a feed.
   ///
   /// This method delegates the ad loading to the appropriate [AdProvider]
@@ -95,6 +139,18 @@ class AdService {
     }
 
     final primaryAdPlatform = adConfig.primaryAdPlatform;
+
+    // If RemoteConfig specifies AdPlatformType.demo but the app is not in demo environment,
+    // log a warning and skip ad load.
+    if (primaryAdPlatform == AdPlatformType.demo &&
+        _environment != AppEnvironment.demo) {
+      _logger.warning(
+        'AdService: RemoteConfig specifies AdPlatformType.demo as primary '
+        'ad platform, but app is not in demo environment. Skipping interstitial ad load.',
+      );
+      return null;
+    }
+
     final adProvider = _adProviders[primaryAdPlatform];
 
     if (adProvider == null) {
@@ -224,6 +280,18 @@ class AdService {
     }
 
     final primaryAdPlatform = adConfig.primaryAdPlatform;
+
+    // If RemoteConfig specifies AdPlatformType.demo but the app is not in demo environment,
+    // log a warning and skip ad load.
+    if (primaryAdPlatform == AdPlatformType.demo &&
+        _environment != AppEnvironment.demo) {
+      _logger.warning(
+        'AdService: RemoteConfig specifies AdPlatformType.demo as primary '
+        'ad platform, but app is not in demo environment. Skipping inline ad load.',
+      );
+      return null;
+    }
+
     final adProvider = _adProviders[primaryAdPlatform];
 
     if (adProvider == null) {
@@ -258,53 +326,69 @@ class AdService {
       return null;
     }
 
-    _logger.info(
-      'AdService: Requesting $adType ad from $primaryAdPlatform AdProvider with ID: $adId '
-      'for ${feedAd ? 'feed' : 'in-article'} placement.',
-    );
-    try {
-      InlineAd? loadedAd;
-      // For in-article banner ads, bannerAdShape dictates the visual style.
-      // For feed ads, headlineImageStyle is still relevant.
-      final effectiveHeadlineImageStyle = feedAd ? headlineImageStyle : null;
-
-      switch (adType) {
-        case AdType.native:
-          loadedAd = await adProvider.loadNativeAd(
-            adPlatformIdentifiers: platformAdIdentifiers,
-            adId: adId,
-            adThemeStyle: adThemeStyle,
-            headlineImageStyle: effectiveHeadlineImageStyle,
-          );
-        case AdType.banner:
-          loadedAd = await adProvider.loadBannerAd(
-            adPlatformIdentifiers: platformAdIdentifiers,
-            adId: adId,
-            adThemeStyle: adThemeStyle,
-            headlineImageStyle: effectiveHeadlineImageStyle,
-          );
-        case AdType.interstitial:
-        case AdType.video:
-          _logger.warning(
-            'AdService: Attempted to load $adType ad using _loadInlineAd. This is not supported.',
-          );
-          return null;
+    for (var attempt = 0; attempt <= _maxAdLoadRetries; attempt++) {
+      if (attempt > 0) {
+        _logger.info(
+          'AdService: Retrying $adType ad load (attempt $attempt) for ID: $adId '
+          'after $_adLoadRetryDelay delay.',
+        );
+        await Future<void>.delayed(_adLoadRetryDelay);
       }
 
-      if (loadedAd != null) {
-        _logger.info('AdService: $adType ad successfully loaded.');
-        return loadedAd;
-      } else {
-        _logger.info('AdService: No $adType ad loaded by AdProvider.');
-        return null;
+      try {
+        _logger.info(
+          'AdService: Requesting $adType ad from $primaryAdPlatform AdProvider with ID: $adId '
+          'for ${feedAd ? 'feed' : 'in-article'} placement.',
+        );
+        InlineAd? loadedAd;
+        // For in-article banner ads, bannerAdShape dictates the visual style.
+        // For feed ads, headlineImageStyle is still relevant.
+        final effectiveHeadlineImageStyle = feedAd ? headlineImageStyle : null;
+
+        switch (adType) {
+          case AdType.native:
+            loadedAd = await adProvider.loadNativeAd(
+              adPlatformIdentifiers: platformAdIdentifiers,
+              adId: adId,
+              adThemeStyle: adThemeStyle,
+              headlineImageStyle: effectiveHeadlineImageStyle,
+            );
+          case AdType.banner:
+            loadedAd = await adProvider.loadBannerAd(
+              adPlatformIdentifiers: platformAdIdentifiers,
+              adId: adId,
+              adThemeStyle: adThemeStyle,
+              headlineImageStyle: effectiveHeadlineImageStyle,
+            );
+          case AdType.interstitial:
+          case AdType.video:
+            _logger.warning(
+              'AdService: Attempted to load $adType ad using _loadInlineAd. This is not supported.',
+            );
+            return null;
+        }
+
+        if (loadedAd != null) {
+          _logger.info('AdService: $adType ad successfully loaded.');
+          return loadedAd;
+        } else {
+          _logger.info('AdService: No $adType ad loaded by AdProvider.');
+          // If no ad is returned, it might be a "no fill" scenario.
+          // Continue to the next retry attempt.
+        }
+      } catch (e, s) {
+        _logger.severe(
+          'AdService: Error getting $adType ad from AdProvider on attempt $attempt: $e',
+          e,
+          s,
+        );
+        // If an exception occurs, log it and continue to the next retry attempt.
       }
-    } catch (e, s) {
-      _logger.severe(
-        'AdService: Error getting $adType ad from AdProvider: $e',
-        e,
-        s,
-      );
-      return null;
     }
+
+    _logger.warning(
+      'AdService: All retry attempts failed for $adType ad with ID: $adId.',
+    );
+    return null;
   }
 }

lib/ads/admob_ad_provider.dart

@@ -88,6 +88,7 @@ class AdMobAdProvider implements AdProvider {
         },
         onAdFailedToLoad: (ad, error) {
           _logger.severe('AdMobAdProvider: Native Ad failed to load: $error');
+          // Ad disposal is now handled by InlineAdCacheService
           completer.complete(null);
         },
         onAdClicked: (ad) {
@@ -119,7 +120,8 @@ class AdMobAdProvider implements AdProvider {
       const Duration(seconds: _adLoadTimeout),
       onTimeout: () {
         _logger.warning('AdMobAdProvider: Native ad loading timed out.');
-        ad.dispose();
+        // Ad disposal is now handled by InlineAdCacheService
+        completer.complete(null);
         return null;
       },
     );
@@ -177,7 +179,7 @@ class AdMobAdProvider implements AdProvider {
         },
         onAdFailedToLoad: (ad, error) {
           _logger.severe('AdMobAdProvider: Banner Ad failed to load: $error');
-          ad.dispose();
+          // Ad disposal is now handled by InlineAdCacheService
           completer.complete(null);
         },
         onAdOpened: (ad) {
@@ -203,7 +205,8 @@ class AdMobAdProvider implements AdProvider {
       const Duration(seconds: _adLoadTimeout),
       onTimeout: () {
         _logger.warning('AdMobAdProvider: Banner ad loading timed out.');
-        ad.dispose();
+        // Ad disposal is now handled by InlineAdCacheService
+        completer.complete(null);
         return null;
       },
     );
@@ -327,4 +330,20 @@ class AdMobAdProvider implements AdProvider {
       ),
     );
   }
+
+  @override
+  Future<void> disposeAd(Object adObject) async {
+    _logger.info('AdMobAdProvider: Attempting to dispose ad object: $adObject');
+    if (adObject is admob.Ad) {
+      await adObject.dispose();
+      _logger.info(
+        'AdMobAdProvider: Disposed AdMob ad object (NativeAd, BannerAd, or InterstitialAd).',
+      );
+    } else {
+      _logger.warning(
+        'AdMobAdProvider: Attempted to dispose a non-AdMob ad object. '
+        'Type: ${adObject.runtimeType}',
+      );
+    }
+  }
 }

lib/ads/demo_ad_provider.dart

@@ -85,4 +85,14 @@ class DemoAdProvider implements AdProvider {
       adObject: Object(), // Placeholder object
     );
   }
+
+  @override
+  Future<void> disposeAd(Object adObject) async {
+    _logger.info(
+      'DemoAdProvider: No explicit disposal needed for demo ad object: '
+      '${adObject.runtimeType}',
+    );
+    // Demo ad objects are simple Dart objects and do not hold native resources
+    // that require explicit disposal.
+  }
 }