refactor(leaderboard_repository): only keep the top-10 score in the leaderboard collection (#265)

packages/leaderboard_repository/lib/src/leaderboard_repository.dart

@@ -1,91 +1,6 @@
 import 'package:cloud_firestore/cloud_firestore.dart';
 import 'package:leaderboard_repository/leaderboard_repository.dart';
 
-/// {@template leaderboard_exception}
-/// Base exception for leaderboard repository failures.
-/// {@endtemplate}
-abstract class LeaderboardException implements Exception {
-  /// {@macro leaderboard_exception}
-  const LeaderboardException(this.error, this.stackTrace);
-
-  /// The error that was caught.
-  final Object error;
-
-  /// The Stacktrace associated with the [error].
-  final StackTrace stackTrace;
-}
-
-/// {@template leaderboard_deserialization_exception}
-/// Exception thrown when leaderboard data cannot be deserialized in the
-/// expected way.
-/// {@endtemplate}
-class LeaderboardDeserializationException extends LeaderboardException {
-  /// {@macro leaderboard_deserialization_exception}
-  const LeaderboardDeserializationException(
-    Object error,
-    StackTrace stackTrace,
-  ) : super(
-          error,
-          stackTrace,
-        );
-}
-
-/// {@template fetch_top_10_leaderboard_exception}
-/// Exception thrown when failure occurs while fetching top 10 leaderboard.
-/// {@endtemplate}
-class FetchTop10LeaderboardException extends LeaderboardException {
-  /// {@macro fetch_top_10_leaderboard_exception}
-  const FetchTop10LeaderboardException(
-    Object error,
-    StackTrace stackTrace,
-  ) : super(
-          error,
-          stackTrace,
-        );
-}
-
-/// {@template add_leaderboard_entry_exception}
-/// Exception thrown when failure occurs while adding entry to leaderboard.
-/// {@endtemplate}
-class AddLeaderboardEntryException extends LeaderboardException {
-  /// {@macro add_leaderboard_entry_exception}
-  const AddLeaderboardEntryException(
-    Object error,
-    StackTrace stackTrace,
-  ) : super(
-          error,
-          stackTrace,
-        );
-}
-
-/// {@template fetch_player_ranking_exception}
-/// Exception thrown when failure occurs while fetching player ranking.
-/// {@endtemplate}
-class FetchPlayerRankingException extends LeaderboardException {
-  /// {@macro fetch_player_ranking_exception}
-  const FetchPlayerRankingException(
-    Object error,
-    StackTrace stackTrace,
-  ) : super(
-          error,
-          stackTrace,
-        );
-}
-
-/// {@template fetch_prohibited_initials_exception}
-/// Exception thrown when failure occurs while fetching prohibited initials.
-/// {@endtemplate}
-class FetchProhibitedInitialsException extends LeaderboardException {
-  /// {@macro fetch_prohibited_initials_exception}
-  const FetchProhibitedInitialsException(
-    Object error,
-    StackTrace stackTrace,
-  ) : super(
-          error,
-          stackTrace,
-        );
-}
-
 /// {@template leaderboard_repository}
 /// Repository to access leaderboard data in Firebase Cloud Firestore.
 /// {@endtemplate}
@@ -97,73 +12,40 @@ class LeaderboardRepository {
 
   final FirebaseFirestore _firebaseFirestore;
 
+  static const _leaderboardLimit = 10;
+  static const _leaderboardCollectionName = 'leaderboard';
+  static const _scoreFieldName = 'score';
+
   /// Acquires top 10 [LeaderboardEntryData]s.
   Future<List<LeaderboardEntryData>> fetchTop10Leaderboard() async {
-    final leaderboardEntries = <LeaderboardEntryData>[];
-    late List<QueryDocumentSnapshot> documents;
-
     try {
       final querySnapshot = await _firebaseFirestore
-          .collection('leaderboard')
-          .orderBy('score', descending: true)
-          .limit(10)
+          .collection(_leaderboardCollectionName)
+          .orderBy(_scoreFieldName, descending: true)
+          .limit(_leaderboardLimit)
           .get();
-      documents = querySnapshot.docs;
+      final documents = querySnapshot.docs;
+      return documents.toLeaderboard();
+    } on LeaderboardDeserializationException {
+      rethrow;
     } on Exception catch (error, stackTrace) {
       throw FetchTop10LeaderboardException(error, stackTrace);
     }
-
-    for (final document in documents) {
-      final data = document.data() as Map<String, dynamic>?;
-      if (data != null) {
-        try {
-          leaderboardEntries.add(LeaderboardEntryData.fromJson(data));
-        } catch (error, stackTrace) {
-          throw LeaderboardDeserializationException(error, stackTrace);
-        }
-      }
-    }
-
-    return leaderboardEntries;
   }
 
-  /// Adds player's score entry to the leaderboard and gets their
-  /// [LeaderboardRanking].
-  Future<LeaderboardRanking> addLeaderboardEntry(
+  /// Adds player's score entry to the leaderboard if it is within the top-10
+  Future<void> addLeaderboardEntry(
     LeaderboardEntryData entry,
   ) async {
-    late DocumentReference entryReference;
-    try {
-      entryReference = await _firebaseFirestore
-          .collection('leaderboard')
-          .add(entry.toJson());
-    } on Exception catch (error, stackTrace) {
-      throw AddLeaderboardEntryException(error, stackTrace);
-    }
-
-    try {
-      final querySnapshot = await _firebaseFirestore
-          .collection('leaderboard')
-          .orderBy('score', descending: true)
-          .get();
-
-      // TODO(allisonryan0002): see if we can find a more performant solution.
-      final documents = querySnapshot.docs;
-      final ranking = documents.indexWhere(
-            (document) => document.id == entryReference.id,
-          ) +
-          1;
-
-      if (ranking > 0) {
-        return LeaderboardRanking(ranking: ranking, outOf: documents.length);
-      } else {
-        throw FetchPlayerRankingException(
-          'Player score could not be found and ranking cannot be provided.',
-          StackTrace.current,
-        );
+    final leaderboard = await _fetchLeaderboardSortedByScore();
+    if (leaderboard.length < 10) {
+      await _saveScore(entry);
+    } else {
+      final tenthPositionScore = leaderboard[9].score;
+      if (entry.score > tenthPositionScore) {
+        await _saveScore(entry);
+        await _deleteScoresUnder(tenthPositionScore);
       }
-    } on Exception catch (error, stackTrace) {
-      throw FetchPlayerRankingException(error, stackTrace);
     }
   }
 
@@ -174,7 +56,6 @@ class LeaderboardRepository {
     if (!initialsRegex.hasMatch(initials)) {
       return false;
     }
-
     try {
       final document = await _firebaseFirestore
           .collection('prohibitedInitials')
@@ -187,4 +68,61 @@ class LeaderboardRepository {
       throw FetchProhibitedInitialsException(error, stackTrace);
     }
   }
+
+  Future<List<LeaderboardEntryData>> _fetchLeaderboardSortedByScore() async {
+    try {
+      final querySnapshot = await _firebaseFirestore
+          .collection(_leaderboardCollectionName)
+          .orderBy(_scoreFieldName, descending: true)
+          .get();
+      final documents = querySnapshot.docs;
+      return documents.toLeaderboard();
+    } on Exception catch (error, stackTrace) {
+      throw FetchLeaderboardException(error, stackTrace);
+    }
+  }
+
+  Future<void> _saveScore(LeaderboardEntryData entry) {
+    try {
+      return _firebaseFirestore
+          .collection(_leaderboardCollectionName)
+          .add(entry.toJson());
+    } on Exception catch (error, stackTrace) {
+      throw AddLeaderboardEntryException(error, stackTrace);
+    }
+  }
+
+  Future<void> _deleteScoresUnder(int score) async {
+    try {
+      final querySnapshot = await _firebaseFirestore
+          .collection(_leaderboardCollectionName)
+          .where(_scoreFieldName, isLessThanOrEqualTo: score)
+          .get();
+      final documents = querySnapshot.docs;
+      for (final document in documents) {
+        await document.reference.delete();
+      }
+    } on LeaderboardDeserializationException {
+      rethrow;
+    } on Exception catch (error, stackTrace) {
+      throw DeleteLeaderboardException(error, stackTrace);
+    }
+  }
+}
+
+extension on List<QueryDocumentSnapshot> {
+  List<LeaderboardEntryData> toLeaderboard() {
+    final leaderboardEntries = <LeaderboardEntryData>[];
+    for (final document in this) {
+      final data = document.data() as Map<String, dynamic>?;
+      if (data != null) {
+        try {
+          leaderboardEntries.add(LeaderboardEntryData.fromJson(data));
+        } catch (error, stackTrace) {
+          throw LeaderboardDeserializationException(error, stackTrace);
+        }
+      }
+    }
+    return leaderboardEntries;
+  }
 }

packages/leaderboard_repository/lib/src/models/exceptions.dart

@@ -0,0 +1,69 @@
+/// {@template leaderboard_exception}
+/// Base exception for leaderboard repository failures.
+/// {@endtemplate}
+abstract class LeaderboardException implements Exception {
+  /// {@macro leaderboard_exception}
+  const LeaderboardException(this.error, this.stackTrace);
+
+  /// The error that was caught.
+  final Object error;
+
+  /// The Stacktrace associated with the [error].
+  final StackTrace stackTrace;
+}
+
+/// {@template leaderboard_deserialization_exception}
+/// Exception thrown when leaderboard data cannot be deserialized in the
+/// expected way.
+/// {@endtemplate}
+class LeaderboardDeserializationException extends LeaderboardException {
+  /// {@macro leaderboard_deserialization_exception}
+  const LeaderboardDeserializationException(Object error, StackTrace stackTrace)
+      : super(error, stackTrace);
+}
+
+/// {@template fetch_top_10_leaderboard_exception}
+/// Exception thrown when failure occurs while fetching top 10 leaderboard.
+/// {@endtemplate}
+class FetchTop10LeaderboardException extends LeaderboardException {
+  /// {@macro fetch_top_10_leaderboard_exception}
+  const FetchTop10LeaderboardException(Object error, StackTrace stackTrace)
+      : super(error, stackTrace);
+}
+
+/// {@template fetch_leaderboard_exception}
+/// Exception thrown when failure occurs while fetching the leaderboard.
+/// {@endtemplate}
+class FetchLeaderboardException extends LeaderboardException {
+  /// {@macro fetch_top_10_leaderboard_exception}
+  const FetchLeaderboardException(Object error, StackTrace stackTrace)
+      : super(error, stackTrace);
+}
+
+/// {@template delete_leaderboard_exception}
+/// Exception thrown when failure occurs while deleting the leaderboard under
+/// the tenth position.
+/// {@endtemplate}
+class DeleteLeaderboardException extends LeaderboardException {
+  /// {@macro fetch_top_10_leaderboard_exception}
+  const DeleteLeaderboardException(Object error, StackTrace stackTrace)
+      : super(error, stackTrace);
+}
+
+/// {@template add_leaderboard_entry_exception}
+/// Exception thrown when failure occurs while adding entry to leaderboard.
+/// {@endtemplate}
+class AddLeaderboardEntryException extends LeaderboardException {
+  /// {@macro add_leaderboard_entry_exception}
+  const AddLeaderboardEntryException(Object error, StackTrace stackTrace)
+      : super(error, stackTrace);
+}
+
+/// {@template fetch_prohibited_initials_exception}
+/// Exception thrown when failure occurs while fetching prohibited initials.
+/// {@endtemplate}
+class FetchProhibitedInitialsException extends LeaderboardException {
+  /// {@macro fetch_prohibited_initials_exception}
+  const FetchProhibitedInitialsException(Object error, StackTrace stackTrace)
+      : super(error, stackTrace);
+}

packages/leaderboard_repository/lib/src/models/leaderboard_ranking.dart

@@ -1,20 +0,0 @@
-import 'package:equatable/equatable.dart';
-import 'package:leaderboard_repository/leaderboard_repository.dart';
-
-/// {@template leaderboard_ranking}
-/// Contains [ranking] for a single [LeaderboardEntryData] and the number of
-/// players the [ranking] is [outOf].
-/// {@endtemplate}
-class LeaderboardRanking extends Equatable {
-  /// {@macro leaderboard_ranking}
-  const LeaderboardRanking({required this.ranking, required this.outOf});
-
-  /// Place ranking by score for a [LeaderboardEntryData].
-  final int ranking;
-
-  /// Number of [LeaderboardEntryData]s at the time of score entry.
-  final int outOf;
-
-  @override
-  List<Object> get props => [ranking, outOf];
-}

packages/leaderboard_repository/lib/src/models/models.dart

@@ -1,2 +1,2 @@
+export 'exceptions.dart';
 export 'leaderboard_entry_data.dart';
-export 'leaderboard_ranking.dart';

packages/leaderboard_repository/test/src/leaderboard_repository_test.dart

@@ -153,7 +153,6 @@ void main() {
         character: CharacterType.dash,
       );
       const entryDocumentId = 'id$entryScore';
-      final ranking = LeaderboardRanking(ranking: 3, outOf: 4);
 
       setUp(() {
         leaderboardRepository = LeaderboardRepository(firestore);
@@ -165,13 +164,12 @@ void main() {
           final queryDocumentSnapshot = MockQueryDocumentSnapshot();
           when(queryDocumentSnapshot.data).thenReturn(<String, dynamic>{
             'character': 'dash',
-            'username': 'user$score',
+            'playerInitials': 'AAA',
             'score': score
           });
           when(() => queryDocumentSnapshot.id).thenReturn('id$score');
           return queryDocumentSnapshot;
         }).toList();
-
         when(() => firestore.collection('leaderboard'))
             .thenAnswer((_) => collectionReference);
         when(() => collectionReference.add(any()))
@@ -184,19 +182,29 @@ void main() {
       });
 
       test(
-          'adds leaderboard entry and returns player ranking when '
-          'firestore operations succeed', () async {
-        final rankingResult =
-            await leaderboardRepository.addLeaderboardEntry(leaderboardEntry);
-
-        expect(rankingResult, equals(ranking));
+          'throws FetchLeaderboardException '
+          'when querying the leaderboard fails', () {
+        when(() => firestore.collection('leaderboard')).thenThrow(Exception());
+        expect(
+          () => leaderboardRepository.addLeaderboardEntry(leaderboardEntry),
+          throwsA(isA<FetchLeaderboardException>()),
+        );
       });
 
       test(
-          'throws AddLeaderboardEntryException when Exception occurs '
-          'when trying to add entry to firestore', () async {
-        when(() => firestore.collection('leaderboard')).thenThrow(Exception());
+          'saves the new score if the existing leaderboard '
+          'has less than 10 scores', () async {
+        await leaderboardRepository.addLeaderboardEntry(leaderboardEntry);
+        verify(
+          () => collectionReference.add(leaderboardEntry.toJson()),
+        ).called(1);
+      });
 
+      test(
+          'throws AddLeaderboardEntryException '
+          'when adding a new entry fails', () async {
+        when(() => collectionReference.add(leaderboardEntry.toJson()))
+            .thenThrow(Exception('oops'));
         expect(
           () => leaderboardRepository.addLeaderboardEntry(leaderboardEntry),
           throwsA(isA<AddLeaderboardEntryException>()),
@@ -204,26 +212,160 @@ void main() {
       });
 
       test(
-          'throws FetchPlayerRankingException when Exception occurs '
-          'when trying to retrieve information from firestore', () async {
-        when(() => collectionReference.orderBy('score', descending: true))
-            .thenThrow(Exception());
+          'does nothing if there are more than 10 scores in the leaderboard '
+          'and the new score is smaller than the top 10', () async {
+        final leaderboardScores = [
+          10000,
+          9500,
+          9000,
+          8500,
+          8000,
+          7500,
+          7000,
+          6500,
+          6000,
+          5500,
+          5000
+        ];
+        final queryDocumentSnapshots = leaderboardScores.map((score) {
+          final queryDocumentSnapshot = MockQueryDocumentSnapshot();
+          when(queryDocumentSnapshot.data).thenReturn(<String, dynamic>{
+            'character': 'dash',
+            'playerInitials': 'AAA',
+            'score': score
+          });
+          when(() => queryDocumentSnapshot.id).thenReturn('id$score');
+          return queryDocumentSnapshot;
+        }).toList();
+        when(() => querySnapshot.docs).thenReturn(queryDocumentSnapshots);
 
-        expect(
-          () => leaderboardRepository.addLeaderboardEntry(leaderboardEntry),
-          throwsA(isA<FetchPlayerRankingException>()),
+        await leaderboardRepository.addLeaderboardEntry(leaderboardEntry);
+        verifyNever(
+          () => collectionReference.add(leaderboardEntry.toJson()),
         );
       });
 
       test(
-          'throws FetchPlayerRankingException when score cannot be found '
-          'in firestore leaderboard data', () async {
-        when(() => documentReference.id).thenReturn('nonexistentDocumentId');
-
+          'throws DeleteLeaderboardException '
+          'when deleting scores outside the top 10 fails', () async {
+        final deleteQuery = MockQuery();
+        final deleteQuerySnapshot = MockQuerySnapshot();
+        final newScore = LeaderboardEntryData(
+          playerInitials: 'ABC',
+          score: 15000,
+          character: CharacterType.android,
+        );
+        final leaderboardScores = [
+          10000,
+          9500,
+          9000,
+          8500,
+          8000,
+          7500,
+          7000,
+          6500,
+          6000,
+          5500,
+          5000,
+        ];
+        final deleteDocumentSnapshots = [5500, 5000].map((score) {
+          final queryDocumentSnapshot = MockQueryDocumentSnapshot();
+          when(queryDocumentSnapshot.data).thenReturn(<String, dynamic>{
+            'character': 'dash',
+            'playerInitials': 'AAA',
+            'score': score
+          });
+          when(() => queryDocumentSnapshot.id).thenReturn('id$score');
+          when(() => queryDocumentSnapshot.reference)
+              .thenReturn(documentReference);
+          return queryDocumentSnapshot;
+        }).toList();
+        when(deleteQuery.get).thenAnswer((_) async => deleteQuerySnapshot);
+        when(() => deleteQuerySnapshot.docs)
+            .thenReturn(deleteDocumentSnapshots);
+        final queryDocumentSnapshots = leaderboardScores.map((score) {
+          final queryDocumentSnapshot = MockQueryDocumentSnapshot();
+          when(queryDocumentSnapshot.data).thenReturn(<String, dynamic>{
+            'character': 'dash',
+            'playerInitials': 'AAA',
+            'score': score
+          });
+          when(() => queryDocumentSnapshot.id).thenReturn('id$score');
+          when(() => queryDocumentSnapshot.reference)
+              .thenReturn(documentReference);
+          return queryDocumentSnapshot;
+        }).toList();
+        when(
+          () => collectionReference.where('score', isLessThanOrEqualTo: 5500),
+        ).thenAnswer((_) => deleteQuery);
+        when(() => documentReference.delete()).thenThrow(Exception('oops'));
+        when(() => querySnapshot.docs).thenReturn(queryDocumentSnapshots);
         expect(
-          () => leaderboardRepository.addLeaderboardEntry(leaderboardEntry),
-          throwsA(isA<FetchPlayerRankingException>()),
+          () => leaderboardRepository.addLeaderboardEntry(newScore),
+          throwsA(isA<DeleteLeaderboardException>()),
+        );
+      });
+
+      test(
+          'saves the new score when there are more than 10 scores in the '
+          'leaderboard and the new score is higher than the lowest top 10, and '
+          'deletes the scores that are not in the top 10 anymore', () async {
+        final deleteQuery = MockQuery();
+        final deleteQuerySnapshot = MockQuerySnapshot();
+        final newScore = LeaderboardEntryData(
+          playerInitials: 'ABC',
+          score: 15000,
+          character: CharacterType.android,
         );
+        final leaderboardScores = [
+          10000,
+          9500,
+          9000,
+          8500,
+          8000,
+          7500,
+          7000,
+          6500,
+          6000,
+          5500,
+          5000,
+        ];
+        final deleteDocumentSnapshots = [5500, 5000].map((score) {
+          final queryDocumentSnapshot = MockQueryDocumentSnapshot();
+          when(queryDocumentSnapshot.data).thenReturn(<String, dynamic>{
+            'character': 'dash',
+            'playerInitials': 'AAA',
+            'score': score
+          });
+          when(() => queryDocumentSnapshot.id).thenReturn('id$score');
+          when(() => queryDocumentSnapshot.reference)
+              .thenReturn(documentReference);
+          return queryDocumentSnapshot;
+        }).toList();
+        when(deleteQuery.get).thenAnswer((_) async => deleteQuerySnapshot);
+        when(() => deleteQuerySnapshot.docs)
+            .thenReturn(deleteDocumentSnapshots);
+        final queryDocumentSnapshots = leaderboardScores.map((score) {
+          final queryDocumentSnapshot = MockQueryDocumentSnapshot();
+          when(queryDocumentSnapshot.data).thenReturn(<String, dynamic>{
+            'character': 'dash',
+            'playerInitials': 'AAA',
+            'score': score
+          });
+          when(() => queryDocumentSnapshot.id).thenReturn('id$score');
+          when(() => queryDocumentSnapshot.reference)
+              .thenReturn(documentReference);
+          return queryDocumentSnapshot;
+        }).toList();
+        when(
+          () => collectionReference.where('score', isLessThanOrEqualTo: 5500),
+        ).thenAnswer((_) => deleteQuery);
+        when(() => documentReference.delete())
+            .thenAnswer((_) async => Future.value());
+        when(() => querySnapshot.docs).thenReturn(queryDocumentSnapshots);
+        await leaderboardRepository.addLeaderboardEntry(newScore);
+        verify(() => collectionReference.add(newScore.toJson())).called(1);
+        verify(() => documentReference.delete()).called(2);
       });
     });
 

packages/leaderboard_repository/test/src/models/leaderboard_ranking_test.dart

@@ -1,19 +0,0 @@
-import 'package:leaderboard_repository/leaderboard_repository.dart';
-import 'package:test/test.dart';
-
-void main() {
-  group('LeaderboardRanking', () {
-    test('can be instantiated', () {
-      const leaderboardRanking = LeaderboardRanking(ranking: 1, outOf: 1);
-
-      expect(leaderboardRanking, isNotNull);
-    });
-
-    test('supports value equality.', () {
-      const leaderboardRanking = LeaderboardRanking(ranking: 1, outOf: 1);
-      const leaderboardRanking2 = LeaderboardRanking(ranking: 1, outOf: 1);
-
-      expect(leaderboardRanking, equals(leaderboardRanking2));
-    });
-  });
-}