fix comments repo. (#364)

lib/config/constants.dart

@@ -83,4 +83,7 @@ abstract class AppDurations {
   static const Duration oneSecond = Duration(seconds: 1);
   static const Duration twoSeconds = Duration(seconds: 2);
   static const Duration tenSeconds = Duration(seconds: 10);
+  static const Duration sec30 = Duration(seconds: 30);
+  static const Duration oneMinute = Duration(minutes: 1);
+  static const Duration twoMinutes = Duration(minutes: 2);
 }

lib/cubits/comments/comments_cubit.dart

@@ -83,6 +83,25 @@ class CommentsCubit extends Cubit<CommentsState> {
   final Map<int, StreamSubscription<Comment>> _streamSubscriptions =
       <int, StreamSubscription<Comment>>{};
 
+  static const int _webFetchingCmtCountLowerLimit = 50;
+
+  Future<bool> get _shouldFetchFromWeb async {
+    final bool isOnWifi = await _isOnWifi;
+    if (isOnWifi) {
+      return switch (state.item) {
+        Story(descendants: final int descendants)
+            when descendants > _webFetchingCmtCountLowerLimit =>
+          true,
+        Comment(kids: final List<int> kids)
+            when kids.length > _webFetchingCmtCountLowerLimit =>
+          true,
+        _ => false,
+      };
+    } else {
+      return true;
+    }
+  }
+
   static Future<bool> get _isOnWifi async {
     final ConnectivityResult status = await Connectivity().checkConnectivity();
     return status == ConnectivityResult.wifi;
@@ -160,8 +179,9 @@ class CommentsCubit extends Cubit<CommentsState> {
         case FetchMode.eager:
           switch (state.order) {
             case CommentsOrder.natural:
-              final bool isOnWifi = await _isOnWifi;
-              if (!isOnWifi && fetchFromWeb) {
+              final bool shouldFetchFromWeb = await _shouldFetchFromWeb;
+              if (fetchFromWeb && shouldFetchFromWeb) {
+                _logger.d('fetching from web.');
                 commentStream = _hackerNewsWebRepository
                     .fetchCommentsStream(state.item)
                     .handleError((dynamic e) {
@@ -170,9 +190,9 @@ class CommentsCubit extends Cubit<CommentsState> {
                   _logger.e(e);
 
                   switch (e.runtimeType) {
+                    case RateLimitedException:
                     case RateLimitedWithFallbackException:
                     case PossibleParsingException:
-                    case BrowserNotRunningException:
                       if (_preferenceCubit.state.devModeEnabled) {
                         onError?.call(e as AppException);
                       }
@@ -184,6 +204,7 @@ class CommentsCubit extends Cubit<CommentsState> {
                   }
                 });
               } else {
+                _logger.d('fetching from API.');
                 commentStream =
                     _hackerNewsRepository.fetchAllCommentsRecursivelyStream(
                   ids: kids,
@@ -256,8 +277,9 @@ class CommentsCubit extends Cubit<CommentsState> {
       case FetchMode.eager:
         switch (state.order) {
           case CommentsOrder.natural:
-            final bool isOnWifi = await _isOnWifi;
-            if (!isOnWifi && fetchFromWeb) {
+            final bool shouldFetchFromWeb = await _shouldFetchFromWeb;
+            if (fetchFromWeb && shouldFetchFromWeb) {
+              _logger.d('fetching from web.');
               commentStream = _hackerNewsWebRepository
                   .fetchCommentsStream(state.item)
                   .handleError((dynamic e) {
@@ -265,8 +287,8 @@ class CommentsCubit extends Cubit<CommentsState> {
 
                 switch (e.runtimeType) {
                   case RateLimitedException:
+                  case RateLimitedWithFallbackException:
                   case PossibleParsingException:
-                  case BrowserNotRunningException:
                     if (_preferenceCubit.state.devModeEnabled) {
                       onError?.call(e as AppException);
                     }
@@ -278,6 +300,7 @@ class CommentsCubit extends Cubit<CommentsState> {
                 }
               });
             } else {
+              _logger.d('fetching from API.');
               commentStream = _hackerNewsRepository
                   .fetchAllCommentsRecursivelyStream(ids: kids);
             }

lib/cubits/notification/notification_cubit.dart

@@ -9,6 +9,7 @@ import 'package:hacki/config/locator.dart';
 import 'package:hacki/cubits/cubits.dart';
 import 'package:hacki/models/models.dart';
 import 'package:hacki/repositories/repositories.dart';
+import 'package:logger/logger.dart';
 
 part 'notification_state.dart';
 
@@ -19,6 +20,7 @@ class NotificationCubit extends Cubit<NotificationState> {
     HackerNewsRepository? hackerNewsRepository,
     PreferenceRepository? preferenceRepository,
     SembastRepository? sembastRepository,
+    Logger? logger,
   })  : _authBloc = authBloc,
         _preferenceCubit = preferenceCubit,
         _hackerNewsRepository =
@@ -27,6 +29,7 @@ class NotificationCubit extends Cubit<NotificationState> {
             preferenceRepository ?? locator.get<PreferenceRepository>(),
         _sembastRepository =
             sembastRepository ?? locator.get<SembastRepository>(),
+        _logger = logger ?? locator.get<Logger>(),
         super(NotificationState.init()) {
     _authBloc.stream
         .map((AuthState event) => event.username)
@@ -58,6 +61,7 @@ class NotificationCubit extends Cubit<NotificationState> {
   final HackerNewsRepository _hackerNewsRepository;
   final PreferenceRepository _preferenceRepository;
   final SembastRepository _sembastRepository;
+  final Logger _logger;
   Timer? _timer;
 
   static const Duration _refreshInterval = Duration(minutes: 5);
@@ -74,6 +78,7 @@ class NotificationCubit extends Cubit<NotificationState> {
     });
 
     await _preferenceRepository.unreadCommentsIds.then((List<int> unreadIds) {
+      _logger.i('NotificationCubit: ${unreadIds.length} unread items.');
       emit(state.copyWith(unreadCommentsIds: unreadIds));
     });
 

lib/models/app_exception.dart

@@ -6,7 +6,7 @@ class AppException implements Exception {
     this.stackTrace,
   });
 
-  final String message;
+  final String? message;
   final StackTrace? stackTrace;
 }
 
@@ -27,10 +27,6 @@ class PossibleParsingException extends AppException {
   final int itemId;
 }
 
-class BrowserNotRunningException extends AppException {
-  BrowserNotRunningException() : super(message: 'Browser not running...');
-}
-
 class GenericException extends AppException {
   GenericException() : super(message: 'Something went wrong...');
 }

lib/models/dio/cached_response.dart

@@ -0,0 +1,19 @@
+import 'package:dio/dio.dart';
+
+class CachedResponse<T> extends Response<T> {
+  CachedResponse({
+    required super.requestOptions,
+    super.data,
+    super.statusCode,
+  }) : setDateTime = DateTime.now();
+
+  factory CachedResponse.fromResponse(Response<T> response) {
+    return CachedResponse<T>(
+      requestOptions: response.requestOptions,
+      data: response.data,
+      statusCode: response.statusCode,
+    );
+  }
+
+  final DateTime setDateTime;
+}

lib/models/preference.dart

@@ -186,7 +186,7 @@ class AutoScrollModePreference extends BooleanPreference {
   String get key => 'autoScrollMode';
 
   @override
-  String get title => 'Auto-scroll on collapsing';
+  String get title => 'Auto-scroll on Collapsing';
 
   @override
   String get subtitle =>

lib/repositories/hacker_news_web_repository.dart

@@ -1,30 +1,36 @@
+import 'dart:async';
 import 'dart:io';
 
 import 'package:collection/collection.dart';
+import 'package:connectivity_plus/connectivity_plus.dart';
 import 'package:dio/dio.dart';
 import 'package:flutter/foundation.dart';
 import 'package:hacki/config/constants.dart';
 import 'package:hacki/models/models.dart';
+import 'package:hacki/utils/utils.dart';
 import 'package:html/dom.dart' hide Comment;
 import 'package:html/parser.dart';
 import 'package:html_unescape/html_unescape.dart';
 
 /// For fetching anything that cannot be fetched through Hacker News API.
 class HackerNewsWebRepository {
-  HackerNewsWebRepository({Dio? dio}) : _dio = dio ?? Dio();
+  HackerNewsWebRepository({
+    Dio? dioWithCache,
+    Dio? dio,
+  })  : _dio = dio ?? Dio(),
+        _dioWithCache = dioWithCache ?? Dio()
+          ..interceptors.addAll(
+            <Interceptor>[
+              if (kDebugMode) LoggerInterceptor(),
+              CacheInterceptor(),
+            ],
+          );
 
+  final Dio _dioWithCache;
   final Dio _dio;
 
   static const Map<String, String> _headers = <String, String>{
-    'accept':
-        'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7',
-    'accept-language': 'en-US,en;q=0.9',
-    'cache-control': 'max-age=0',
-    'sec-fetch-dest': 'document',
-    'sec-fetch-mode': 'navigate',
-    'sec-fetch-site': 'same-origin',
-    'sec-fetch-user': '?1',
-    'upgrade-insecure-requests': '1',
+    'accept': '*/*',
     'user-agent':
         'Mozilla/5.0 (iPhone; CPU iPhone OS 17_1_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.1 Mobile/15E148 Safari/604.1',
   };
@@ -35,6 +41,7 @@ class HackerNewsWebRepository {
       '#hnmain > tbody > tr:nth-child(3) > td > table > tbody > .athing';
 
   Future<Iterable<int>> fetchFavorites({required String of}) async {
+    final bool isOnWifi = await _isOnWifi;
     final String username = of;
     final List<int> allIds = <int>[];
     int page = 1;
@@ -45,7 +52,8 @@ class HackerNewsWebRepository {
         final Uri url = Uri.parse(
           '''$_favoritesBaseUrl$username${isComment ? '&comments=t' : ''}&p=$page''',
         );
-        final Response<String> response = await _dio.getUri<String>(url);
+        final Response<String> response =
+            await (isOnWifi ? _dioWithCache : _dio).getUri<String>(url);
 
         /// Due to rate limiting, we have a short break here.
         await Future<void>.delayed(AppDurations.twoSeconds);
@@ -89,7 +97,7 @@ class HackerNewsWebRepository {
 
   static const String _itemBaseUrl = 'https://news.ycombinator.com/item?id=';
   static const String _athingComtrSelector =
-      '#hnmain > tbody > tr:nth-child(3) > td > table > tbody > .athing.comtr';
+      '#hnmain > tbody > tr > td > table > tbody > .athing.comtr';
   static const String _commentTextSelector =
       '''td > table > tbody > tr > td.default > div.comment''';
   static const String _commentHeadSelector =
@@ -100,6 +108,7 @@ class HackerNewsWebRepository {
       '''td > table > tbody > tr > td.ind''';
 
   Stream<Comment> fetchCommentsStream(Item item) async* {
+    final bool isOnWifi = await _isOnWifi;
     final int itemId = item.id;
     final int? descendants = item is Story ? item.descendants : null;
     int parentTextCount = 0;
@@ -111,10 +120,14 @@ class HackerNewsWebRepository {
           headers: _headers,
           persistentConnection: true,
         );
-        final Response<String> response = await _dio.getUri<String>(
+
+        /// Be more conservative while user is on wifi.
+        final Response<String> response =
+            await (isOnWifi ? _dioWithCache : _dio).getUri<String>(
           url,
           options: option,
         );
+
         final String data = response.data ?? '';
 
         if (page == 1) {
@@ -140,6 +153,10 @@ class HackerNewsWebRepository {
     Iterable<Element> elements = await fetchElements(page);
     final Map<int, int> indentToParentId = <int, int>{};
 
+    if (item is Story && item.descendants > 0 && elements.isEmpty) {
+      throw PossibleParsingException(itemId: itemId);
+    }
+
     while (elements.isNotEmpty) {
       for (final Element element in elements) {
         /// Get comment id.
@@ -228,6 +245,11 @@ class HackerNewsWebRepository {
     }
   }
 
+  static Future<bool> get _isOnWifi async {
+    final ConnectivityResult status = await Connectivity().checkConnectivity();
+    return status == ConnectivityResult.wifi;
+  }
+
   static Future<String> _parseCommentTextHtml(String text) async {
     return HtmlUnescape()
         .convert(text)

lib/repositories/postable_repository.dart

@@ -5,7 +5,6 @@ import 'package:flutter/material.dart';
 import 'package:hacki/models/models.dart';
 import 'package:hacki/repositories/auth_repository.dart';
 import 'package:hacki/repositories/post_repository.dart';
-import 'package:hacki/utils/service_exception.dart';
 
 /// [PostableRepository] is solely for hosting functionalities shared between
 /// [AuthRepository] and [PostRepository].
@@ -40,7 +39,7 @@ class PostableRepository {
       }
 
       return true;
-    } on ServiceException {
+    } on AppException {
       return false;
     }
   }
@@ -65,7 +64,7 @@ class PostableRepository {
         ),
       );
     } on DioException catch (e) {
-      throw ServiceException(e.message);
+      throw AppException(message: e.message);
     }
   }
 

lib/screens/home/home_screen.dart

@@ -49,7 +49,7 @@ class _HomeScreenState extends State<HomeScreen>
     super.didPopNext();
     if (context.read<StoriesBloc>().deviceScreenType ==
         DeviceScreenType.mobile) {
-      locator.get<Logger>().i('Resetting comments in CommentCache');
+      locator.get<Logger>().i('resetting comments in CommentCache');
       Future<void>.delayed(
         AppDurations.ms500,
         locator.get<CommentCache>().resetComments,

lib/screens/item/widgets/main_view.dart

@@ -511,6 +511,9 @@ class _ParentItemSection extends StatelessWidget {
                 style: TextStyle(color: Palette.grey),
               ),
             ),
+            const SizedBox(
+              height: 120,
+            ),
           ],
         ],
       ),

lib/services/web_analyzer.dart

@@ -120,7 +120,7 @@ class WebAnalyzer {
 
     if (info != null) {
       locator.get<Logger>().d('''
-Fetched mem cached metadata using key $key for $story:
+fetched mem cached metadata using key $key for $story:
 ${info.toJson()}
           ''');
       return info;
@@ -168,7 +168,7 @@ ${info.toJson()}
       /// [5] If there is file cache, move it to mem cache for later retrieval.
       if (info != null) {
         locator.get<Logger>().d('''
-Fetched file cached metadata using key $key for $story:
+fetched file cached metadata using key $key for $story:
 ${info.toJson()}
 ''');
         cacheMap[key] = info;
@@ -189,7 +189,7 @@ ${info.toJson()}
         if (info is WebInfo) {
           locator
               .get<Logger>()
-              .d('Caching metadata using key $key for $story.');
+              .d('caching metadata using key $key for $story.');
           unawaited(
             locator.get<SembastRepository>().cacheMetadata(
                   key: key,
@@ -422,7 +422,7 @@ ${info.toJson()}
         } catch (e) {
           locator
               .get<Logger>()
-              .e('''Web page resolution failure from:$url Error:$e''');
+              .e('''web page resolution failure from:$url Error:$e''');
         }
       }
 

lib/utils/dio_interceptors/cache_interceptor.dart

@@ -0,0 +1,46 @@
+import 'dart:io';
+
+import 'package:dio/dio.dart';
+import 'package:hacki/config/constants.dart';
+import 'package:hacki/models/dio/cached_response.dart';
+
+class CacheInterceptor extends InterceptorsWrapper {
+  CacheInterceptor()
+      : super(
+          onResponse: (
+            Response<dynamic> response,
+            ResponseInterceptorHandler handler,
+          ) async {
+            final String key = response.requestOptions.uri.toString();
+
+            if (response.statusCode == HttpStatus.ok) {
+              final CachedResponse<dynamic> cachedResponse =
+                  CachedResponse<dynamic>.fromResponse(response);
+              _cache[key] = cachedResponse;
+            }
+
+            return handler.next(response);
+          },
+          onRequest: (
+            RequestOptions options,
+            RequestInterceptorHandler handler,
+          ) async {
+            final String key = options.uri.toString();
+            final CachedResponse<dynamic>? cachedResponse = _cache[key];
+
+            if (cachedResponse != null &&
+                DateTime.now()
+                        .difference(cachedResponse.setDateTime)
+                        .inSeconds <
+                    _delay.inSeconds) {
+              return handler.resolve(cachedResponse);
+            }
+
+            return handler.next(options);
+          },
+        );
+
+  static const Duration _delay = AppDurations.oneMinute;
+  static final Map<String, CachedResponse<dynamic>> _cache =
+      <String, CachedResponse<dynamic>>{};
+}

lib/utils/dio_interceptors/interceptors.dart

@@ -0,0 +1,2 @@
+export 'cache_interceptor.dart';
+export 'logger_interceptor.dart';

lib/utils/dio_interceptors/logger_interceptor.dart

@@ -0,0 +1,14 @@
+import 'package:pretty_dio_logger/pretty_dio_logger.dart';
+
+class LoggerInterceptor extends PrettyDioLogger {
+  LoggerInterceptor()
+      : super(
+          requestHeader: true,
+          requestBody: true,
+          responseBody: false,
+          responseHeader: true,
+          error: true,
+          compact: true,
+          maxWidth: 90,
+        );
+}

lib/utils/service_exception.dart

@@ -1,14 +0,0 @@
-class ServiceException implements Exception {
-  ServiceException([this.message]);
-
-  final String? message;
-
-  @override
-  String toString() {
-    String result = 'ServiceException';
-    if (message != null) {
-      result = '$result: $message';
-    }
-    return result;
-  }
-}

lib/utils/utils.dart

@@ -1,9 +1,9 @@
 export 'debouncer.dart';
+export 'dio_interceptors/interceptors.dart';
 export 'haptic_feedback_util.dart';
 export 'html_util.dart';
 export 'link_util.dart';
 export 'linkifier_util.dart';
 export 'log_util.dart';
-export 'service_exception.dart';
 export 'theme_util.dart';
 export 'throttle.dart';

pubspec.lock

@@ -815,6 +815,14 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "1.5.1"
+  pretty_dio_logger:
+    dependency: "direct main"
+    description:
+      name: pretty_dio_logger
+      sha256: "00b80053063935cf9a6190da344c5373b9d0e92da4c944c878ff2fbef0ef6dc2"
+      url: "https://pub.dev"
+    source: hosted
+    version: "1.3.1"
   process:
     dependency: transitive
     description:
@@ -1446,4 +1454,4 @@ packages:
     version: "3.1.2"
 sdks:
   dart: ">=3.2.0-194.0.dev <4.0.0"
-  flutter: ">=3.16.3"
+  flutter: ">=3.16.5"

pubspec.yaml

@@ -1,11 +1,11 @@
 name: hacki
 description: A Hacker News reader.
-version: 2.6.0+135
+version: 2.6.2+137
 publish_to: none
 
 environment:
   sdk: ">=3.0.0 <4.0.0"
-  flutter: "3.16.3"
+  flutter: "3.16.5"
 
 dependencies:
   adaptive_theme: ^3.2.0
@@ -57,6 +57,7 @@ dependencies:
   path_provider: ^2.0.12
   path_provider_android: ^2.0.22
   path_provider_foundation: ^2.1.1
+  pretty_dio_logger: ^1.3.1
   pull_to_refresh:
     git:
       url: https://github.com/livinglist/flutter_pulltorefresh