logic changes and code refactoring on search_service.dart

lib/models/search/location_api_response.dart

@@ -0,0 +1,52 @@
+class LocationApiResponse {
+  final List<LocationDataFromResponse> results;
+  LocationApiResponse({
+    this.results,
+  });
+
+  LocationApiResponse copyWith({
+    List<LocationDataFromResponse> results,
+  }) {
+    return LocationApiResponse(
+      results: results ?? this.results,
+    );
+  }
+
+  factory LocationApiResponse.fromMap(Map<String, dynamic> map) {
+    return LocationApiResponse(
+      results: List<LocationDataFromResponse>.from(
+        (map['results']).map(
+          (x) => LocationDataFromResponse.fromMap(x as Map<String, dynamic>),
+        ),
+      ),
+    );
+  }
+}
+
+class LocationDataFromResponse {
+  final String place;
+  final List<double> bbox;
+  LocationDataFromResponse({
+    this.place,
+    this.bbox,
+  });
+
+  LocationDataFromResponse copyWith({
+    String place,
+    List<double> bbox,
+  }) {
+    return LocationDataFromResponse(
+      place: place ?? this.place,
+      bbox: bbox ?? this.bbox,
+    );
+  }
+
+  factory LocationDataFromResponse.fromMap(Map<String, dynamic> map) {
+    return LocationDataFromResponse(
+      place: map['place'] as String,
+      bbox: List<double>.from(
+        (map['bbox']),
+      ),
+    );
+  }
+}

lib/models/search/location_misc./place_and_bbox.dart

@@ -1,27 +0,0 @@
-class PlaceAndBbox {
-  final String place;
-  final List<double> bbox;
-  PlaceAndBbox({
-    this.place,
-    this.bbox,
-  });
-
-  PlaceAndBbox copyWith({
-    String place,
-    List<double> bbox,
-  }) {
-    return PlaceAndBbox(
-      place: place ?? this.place,
-      bbox: bbox ?? this.bbox,
-    );
-  }
-
-  factory PlaceAndBbox.fromMap(Map<String, dynamic> map) {
-    return PlaceAndBbox(
-      place: map['place'] as String,
-      bbox: List<double>.from(
-        (map['bbox']),
-      ),
-    );
-  }
-}

lib/models/search/location_misc./results_to_list_of_place_and_bbox.dart

@@ -1,26 +0,0 @@
-import 'package:photos/models/search/location_misc./place_and_bbox.dart';
-
-class ResultsToListOfPlaceAndBbox {
-  List<PlaceAndBbox> results;
-  ResultsToListOfPlaceAndBbox({
-    this.results,
-  });
-
-  ResultsToListOfPlaceAndBbox copyWith({
-    List<PlaceAndBbox> results,
-  }) {
-    return ResultsToListOfPlaceAndBbox(
-      results: results ?? this.results,
-    );
-  }
-
-  factory ResultsToListOfPlaceAndBbox.fromMap(Map<String, dynamic> map) {
-    return ResultsToListOfPlaceAndBbox(
-      results: List<PlaceAndBbox>.from(
-        (map['results']).map(
-          (x) => PlaceAndBbox.fromMap(x as Map<String, dynamic>),
-        ),
-      ),
-    );
-  }
-}

lib/services/collections_service.dart

@@ -72,10 +72,6 @@ class CollectionsService {
     });
   }
 
-  Map<int, Collection> getCollectionIDtoCollections() {
-    return _collectionIDToCollections;
-  }
-
   Future<List<Collection>> sync() async {
     _logger.info("Syncing collections");
     final lastCollectionUpdationTime =

lib/services/search_service.dart

@@ -1,3 +1,5 @@
+import 'dart:math';
+
 import 'package:dio/dio.dart';
 import 'package:logging/logging.dart';
 import 'package:photos/core/configuration.dart';
@@ -9,8 +11,7 @@ import 'package:photos/models/collection.dart';
 import 'package:photos/models/collection_items.dart';
 import 'package:photos/models/file.dart';
 import 'package:photos/models/location.dart';
-import 'package:photos/models/search/location_misc./place_and_bbox.dart';
-import 'package:photos/models/search/location_misc./results_to_list_of_place_and_bbox.dart';
+import 'package:photos/models/search/location_api_response.dart';
 import 'package:photos/models/search/location_search_result.dart';
 import 'package:photos/services/collections_service.dart';
 import 'package:photos/services/user_service.dart';
@@ -30,12 +31,11 @@ class SearchService {
   Future<void> init() async {
     // Intention of delay is to give more CPU cycles to other tasks
     Future.delayed(const Duration(seconds: 5), () async {
-      // In case home screen loads before 5 seconds and user starts search, future will not be null
-      _future == null
-          ? getAllFiles().then((value) {
-              _cachedFiles = value;
-            })
-          : null;
+      /* In case home screen loads before 5 seconds and user starts search,
+       future will not be null.So here getAllFiles won't run again in that case. */
+      if (_future == null) {
+        getAllFiles();
+      }
     });
 
     Bus.instance.on<LocalPhotosUpdatedEvent>().listen((event) {
@@ -55,19 +55,15 @@ class SearchService {
     return _future;
   }
 
-  Future<List<File>> _fetchAllFiles() async {
-    _cachedFiles = await FilesDB.instance.getAllFilesFromDB();
-    return _cachedFiles;
-  }
-
   Future<List<File>> getFilesOnFilenameSearch(String query) async {
     final List<File> matchedFiles = [];
     final List<File> files = await getAllFiles();
+    final nonCaseSensitiveRegexForQuery = RegExp(query, caseSensitive: false);
     for (int i = 0;
         (i < files.length) && (matchedFiles.length < _maximumResultsLimit);
         i++) {
       File file = files[i];
-      if (file.title.contains(RegExp(query, caseSensitive: false))) {
+      if (file.title.contains(nonCaseSensitiveRegexForQuery)) {
         matchedFiles.add(file);
       }
     }
@@ -78,12 +74,12 @@ class SearchService {
     _cachedFiles.clear();
   }
 
-  Future<List<LocationSearchResult>> getLocationsAndMatchedFiles(
+  Future<List<LocationSearchResult>> getLocationSearchResults(
     String query,
   ) async {
     try {
       final List<File> allFiles = await SearchService.instance.getAllFiles();
-      final List<LocationSearchResult> locationsAndMatchedFiles = [];
+      final List<LocationSearchResult> locationSearchResults = [];
 
       final response = await _dio.get(
         _config.getHttpEndpoint() + "/search/location",
@@ -93,42 +89,32 @@ class SearchService {
         ),
       );
 
-      final matchedLocationNamesAndBboxs =
-          ResultsToListOfPlaceAndBbox.fromMap(response.data);
+      final matchedLocationSearchResults =
+          LocationApiResponse.fromMap(response.data);
+
+      for (LocationDataFromResponse locationData
+          in matchedLocationSearchResults.results) {
+        final List<File> filesInLocation = [];
 
-      for (PlaceAndBbox locationAndBbox
-          in matchedLocationNamesAndBboxs.results) {
-        locationsAndMatchedFiles.add(
-          LocationSearchResult(locationAndBbox.place, []),
-        );
         for (File file in allFiles) {
-          if (_isValidLocation(file.location)) {
-            //format returned by the api is [lng,lat,lng,lat] where indexes 0 & 1 are southwest and 2 & 3 northeast
-            if (file.location.longitude > locationAndBbox.bbox[0] &&
-                file.location.latitude > locationAndBbox.bbox[1] &&
-                file.location.longitude < locationAndBbox.bbox[2] &&
-                file.location.latitude < locationAndBbox.bbox[3]) {
-              locationsAndMatchedFiles.last.files.add(file);
-            }
+          if (_isValidLocation(file.location) &&
+              _isLocationWithinBounds(file.location, locationData)) {
+            filesInLocation.add(file);
           }
         }
+        if (filesInLocation.isNotEmpty) {
+          locationSearchResults.add(
+            LocationSearchResult(locationData.place, filesInLocation),
+          );
+        }
       }
-      locationsAndMatchedFiles.removeWhere((e) => e.files.isEmpty);
-      return locationsAndMatchedFiles;
+      return locationSearchResults;
     } on DioError catch (e) {
       _logger.info(e);
       rethrow;
     }
   }
 
-  bool _isValidLocation(Location location) {
-    return location != null &&
-        location.latitude != null &&
-        location.latitude != 0 &&
-        location.longitude != null &&
-        location.longitude != 0;
-  }
-
   // getFilteredCollectionsWithThumbnail removes deleted or archived or
   // collections which don't have a file from search result
   Future<List<CollectionWithThumbnail>> getFilteredCollectionsWithThumbnail(
@@ -136,38 +122,28 @@ class SearchService {
   ) async {
     // identify collections which have at least one file as we don't display
     // empty collection
-
+    final nonCaseSensitiveRegexForQuery = RegExp(query, caseSensitive: false);
     final List<File> latestCollectionFiles =
         await _collectionService.getLatestCollectionFiles();
     final Map<int, File> collectionIDToLatestFileMap = {
       for (File file in latestCollectionFiles) file.collectionID: file
     };
 
-    /* Identify collections whose name matches the search query
-      and is not archived
-      and is not deleted
-      and has at-least one file
-     */
-
-    final List<Collection> matchedCollection = _collectionService
-        .getCollectionIDtoCollections()
-        .values
+    final List<Collection> matchedCollections = _collectionService
+        .getActiveCollections()
         .where(
           (c) =>
-              !c.isDeleted && // not deleted
               !c.isArchived() // not archived
               &&
               collectionIDToLatestFileMap.containsKey(c.id) && // the
               // collection is not empty
-              c.name.contains(RegExp(query, caseSensitive: false)),
+              c.name.contains(nonCaseSensitiveRegexForQuery),
         )
         .toList();
     final List<CollectionWithThumbnail> result = [];
-    final limit = matchedCollection.length < _maximumResultsLimit
-        ? matchedCollection.length
-        : _maximumResultsLimit;
+    final limit = min(matchedCollections.length, _maximumResultsLimit);
     for (int i = 0; i < limit; i++) {
-      Collection collection = matchedCollection[i];
+      Collection collection = matchedCollections[i];
       result.add(
         CollectionWithThumbnail(
           collection,
@@ -177,4 +153,28 @@ class SearchService {
     }
     return result;
   }
+
+  bool _isValidLocation(Location location) {
+    return location != null &&
+        location.latitude != null &&
+        location.latitude != 0 &&
+        location.longitude != null &&
+        location.longitude != 0;
+  }
+
+  bool _isLocationWithinBounds(
+    Location location,
+    LocationDataFromResponse locationData,
+  ) {
+    //format returned by the api is [lng,lat,lng,lat] where indexes 0 & 1 are southwest and 2 & 3 northeast
+    return location.longitude > locationData.bbox[0] &&
+        location.latitude > locationData.bbox[1] &&
+        location.longitude < locationData.bbox[2] &&
+        location.latitude < locationData.bbox[3];
+  }
+
+  Future<List<File>> _fetchAllFiles() async {
+    _cachedFiles = await FilesDB.instance.getAllFilesFromDB();
+    return _cachedFiles;
+  }
 }

lib/ui/viewer/search/search_widget.dart

@@ -141,7 +141,7 @@ class _SearchWidgetState extends State<SearchWidget> {
       allResults.add(AlbumSearchResult(collectionResult));
     }
     final locationResults =
-        await SearchService.instance.getLocationsAndMatchedFiles(query);
+        await SearchService.instance.getLocationSearchResults(query);
     for (LocationSearchResult result in locationResults) {
       allResults.add(result);
     }