fix: refactor updateAtTopOfLoop

Got rid of the ensureUpdated* calls in AgentCache and ShipCache. I still need to delete a bunch of in-memory caches and use the db in more places. There are likely still bugs here.
eseidel · Feb 24, 2024 · 2391bc6 · 2391bc6
1 parent ff0848a
commit 2391bc6
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 98 deletions.
diff --git a/packages/cli/bin/behavior.dart b/packages/cli/bin/behavior.dart
@@ -12,14 +12,14 @@ Future<void> command(FileSystem fs, Database db, ArgResults argResults) async {
         .toList();
   }
   if (behaviors.isEmpty) {
-    print('No behaviors found.');
+    logger.info('No behaviors found.');
     return;
   }
 
   final jsonList = behaviors.map((b) => b.toJson()).toList();
   const encoder = JsonEncoder.withIndent(' ');
   final prettyprint = encoder.convert(jsonList);
-  print(prettyprint);
+  logger.info(prettyprint);
 }
 
 void main(List<String> args) async {

diff --git a/packages/cli/lib/cache/agent_cache.dart b/packages/cli/lib/cache/agent_cache.dart
@@ -9,7 +9,7 @@ import 'package:cli/net/queries.dart';
 /// is a holder for that object.
 class AgentCache {
   /// Creates a new ship cache.
-  AgentCache(Agent agent, Database db, {this.requestsBetweenChecks = 100})
+  AgentCache(Agent agent, Database db)
       : _agent = agent,
         _db = db;
 
@@ -45,11 +45,6 @@ class AgentCache {
     await _db.upsertAgent(agent);
   }
 
-  /// Number of requests between checks to ensure ships are up to date.
-  final int requestsBetweenChecks;
-
-  int _requestsSinceLastCheck = 0;
-
   /// The headquarters of the agent.
   SystemWaypoint headquarters(SystemsCache systems) =>
       systems.waypoint(agent.headquarters);
@@ -59,20 +54,4 @@ class AgentCache {
 
   /// The symbol of the system of the agent's headquarters.
   SystemSymbol get headquartersSystemSymbol => agent.headquarters.system;
-
-  /// Ensures the agent in the cache is up to date.
-  // TODO(eseidel): Move this out of this class.
-  Future<void> ensureAgentUpToDate(Api api) async {
-    _requestsSinceLastCheck++;
-    if (_requestsSinceLastCheck < requestsBetweenChecks) {
-      return;
-    }
-    final newAgent = await getMyAgent(api);
-    _requestsSinceLastCheck = 0;
-    if (newAgent == agent) {
-      return;
-    }
-    logger.warn('Agent changed, updating cache.');
-    await updateAgent(newAgent);
-  }
 }
diff --git a/packages/cli/lib/cache/caches.dart b/packages/cli/lib/cache/caches.dart
@@ -13,6 +13,8 @@ import 'package:cli/cache/shipyard_listing_cache.dart';
 import 'package:cli/cache/static_cache.dart';
 import 'package:cli/cache/systems_cache.dart';
 import 'package:cli/cache/waypoint_cache.dart';
+import 'package:cli/compare.dart';
+import 'package:cli/logger.dart';
 import 'package:cli/nav/navigation.dart';
 import 'package:cli/nav/route.dart';
 import 'package:cli/nav/system_connectivity.dart';
@@ -189,25 +191,71 @@ class Caches {
     );
     routePlanner.clearRoutingCaches();
   }
+}
+
+T _checkForChanges<T>({
+  required T current,
+  required T server,
+  required List<Map<String, dynamic>> Function(T) toJsonList,
+}) {
+  final currentJson = toJsonList(current);
+  final serverJson = toJsonList(server);
+  if (!jsonMatches(currentJson, serverJson)) {
+    logger.warn('$T changed, updating cache.');
+    return server;
+  }
+  return current;
+}
+
+/// Class to hold state for updating caches at the top of the loop.
+class TopOfLoopUpdater {
+  /// Number of requests between checks to ensure ships are up to date.
+  final int requestsBetweenChecks = 100;
+
+  int _requestsSinceLastCheck = 0;
 
   /// Update the caches at the top of the loop.
-  Future<void> updateAtTopOfLoop(Database db, Api api) async {
+  Future<void> updateAtTopOfLoop(Caches caches, Database db, Api api) async {
     // MarketCache only live for one loop over the ships.
-    markets.resetForLoop();
+    caches.markets.resetForLoop();
 
-    // This check races with the code in continueNavigationIfNeeded which
+    // The ships check races with the code in continueNavigationIfNeeded which
     // knows how to update the ShipNavStatus from IN_TRANSIT to IN_ORBIT when
     // a ship has arrived.  We could add some special logic here to ignore
     // that false positive.  This check is called at the top of every loop
     // and might notice that a ship has arrived before the ship logic gets
     // to run and update the status.
-    ships = await ships.ensureUpToDate(db, api);
-    await agent.ensureAgentUpToDate(api);
-    await fetchContracts(db, api);
-    marketPrices = await MarketPriceSnapshot.load(db);
+    _requestsSinceLastCheck++;
+    if (_requestsSinceLastCheck >= requestsBetweenChecks) {
+      _requestsSinceLastCheck = 0;
+      // This does not need to assign to anything, fetchContracts updates
+      // the db already.
+      _checkForChanges(
+        current: await ContractSnapshot.load(db),
+        server: await fetchContracts(db, api),
+        toJsonList: (e) => e.contracts.map((e) => e.toJson()).toList(),
+      );
+      // caches.ships should be deleted.
+      caches.ships = _checkForChanges(
+        current: caches.ships,
+        server: await fetchShips(db, api),
+        toJsonList: (e) => e.ships.map((e) => e.toJson()).toList(),
+      );
+      // caches.agent should be deleted.
+      await caches.agent.updateAgent(
+        _checkForChanges(
+          current: caches.agent.agent,
+          server: await getMyAgent(api),
+          toJsonList: (e) => [e.toOpenApi().toJson()],
+        ),
+      );
+    }
 
-    marketListings = await MarketListingSnapshot.load(db);
-    shipyardListings = await ShipyardListingSnapshot.load(db);
+    // These should all be deleted from Caches.
+    caches
+      ..marketPrices = await MarketPriceSnapshot.load(db)
+      ..marketListings = await MarketListingSnapshot.load(db)
+      ..shipyardListings = await ShipyardListingSnapshot.load(db);
   }
 }
 

diff --git a/packages/cli/lib/cache/ship_cache.dart b/packages/cli/lib/cache/ship_cache.dart
@@ -1,7 +1,5 @@
 import 'package:cli/api.dart';
 import 'package:cli/cache/static_cache.dart';
-import 'package:cli/compare.dart';
-import 'package:cli/logger.dart';
 import 'package:cli/net/queries.dart';
 import 'package:cli/ships.dart';
 import 'package:collection/collection.dart';
@@ -24,11 +22,6 @@ class ShipSnapshot {
   /// Ships in the cache.
   final List<Ship> ships;
 
-  /// Number of requests between checks to ensure ships are up to date.
-  final int requestsBetweenChecks = 100;
-
-  int _requestsSinceLastCheck = 0;
-
   /// Returns a map of ship frame type to count in fleet.
   // TODO(eseidel): Unclear if this is still needed.
   Map<ShipFrameSymbolEnum, int> get frameCounts => countFrames(ships);
@@ -82,27 +75,6 @@ class ShipSnapshot {
   Ship operator [](ShipSymbol symbol) =>
       ships.firstWhere((s) => s.shipSymbol == symbol);
 
-  /// Fetches a new snapshot and logs if different from this one.
-  // TODO(eseidel): This does not belong in this class.
-  Future<ShipSnapshot> ensureUpToDate(Database db, Api api) async {
-    _requestsSinceLastCheck++;
-    if (_requestsSinceLastCheck < requestsBetweenChecks) {
-      return this;
-    }
-    _requestsSinceLastCheck = 0;
-
-    final newShips = await fetchShips(db, api);
-    final newShipsJson = newShips.ships.map((c) => c.toJson()).toList();
-    final oldShipsJson = ships.map((c) => c.toJson()).toList();
-    // Our contracts class has a timestamp which we don't want to compare, so
-    // compare the OpenAPI JSON instead.
-    if (jsonMatches(newShipsJson, oldShipsJson)) {
-      logger.warn('Contracts changed, updating cache.');
-      return newShips;
-    }
-    return this;
-  }
-
   // TODO(eseidel): This should not exist.  We don't need to pass the
   // ShipSnapshot around everywhere we should just pass the ship and then update
   // the ship when we're done in the db.

diff --git a/packages/cli/lib/logic.dart b/packages/cli/lib/logic.dart
@@ -2,7 +2,6 @@ import 'package:cli/behavior/advance.dart';
 import 'package:cli/behavior/central_command.dart';
 import 'package:cli/cache/caches.dart';
 import 'package:cli/config.dart';
-import 'package:cli/idle_queue.dart';
 import 'package:cli/logger.dart';
 import 'package:cli/net/counts.dart';
 import 'package:cli/net/exceptions.dart';
@@ -25,42 +24,14 @@ Future<void> _waitIfNeeded(ShipWaiterEntry entry) async {
   }
 }
 
-Future<void> _runIdleTasksIfPossible(
-  Database db,
-  Api api,
-  Caches caches,
-  IdleQueue queue,
-  ShipWaiterEntry entry,
-) async {
-  if (!config.serviceIdleQueue) {
-    return;
-  }
-  final waitUntil = entry.waitUntil;
-  if (waitUntil == null) {
-    return;
-  }
-  if (queue.isDone) {
-    return;
-  }
-  while (!queue.isDone &&
-      DateTime.timestamp().add(queue.minProcessingTime).isBefore(waitUntil)) {
-    await expectTime(
-      api.requestCounts,
-      'idle queue',
-      queue.minProcessingTime,
-      () async => await queue.runOne(db, api, caches),
-    );
-  }
-}
-
 /// Loop over our ships and advance them.  Runs until error.
 Future<void> advanceShips(
   Api api,
   Database db,
   CentralCommand centralCommand,
   Caches caches,
-  IdleQueue queue,
-  ShipWaiter waiter, {
+  ShipWaiter waiter,
+  TopOfLoopUpdater updater, {
   required int loopCount,
   bool Function(Ship ship)? shipFilter,
 }) async {
@@ -69,8 +40,7 @@ Future<void> advanceShips(
 
   await expectTime(api.requestCounts, 'top of loop', const Duration(seconds: 1),
       () async {
-    logger.info('🔎 $queue');
-    await caches.updateAtTopOfLoop(db, api);
+    await updater.updateAtTopOfLoop(caches, db, api);
     await centralCommand.advanceCentralPlanning(db, api, caches);
   });
 
@@ -85,7 +55,6 @@ Future<void> advanceShips(
     final shipSymbol = entry.shipSymbol;
     final waitUntil = entry.waitUntil;
 
-    await _runIdleTasksIfPossible(db, api, caches, queue, entry);
     await _waitIfNeeded(entry);
     final ship = caches.ships[shipSymbol];
     if (shipFilter != null && !shipFilter(ship)) {
@@ -206,8 +175,7 @@ Future<Never> logic(
       shipFilter: shipFilter,
     );
   final rateLimitTracker = RateLimitTracker(api);
-  final queue = IdleQueue()
-    ..queueSystem(caches.agent.headquartersSystemSymbol, jumpDistance: 0);
+  final updater = TopOfLoopUpdater();
 
   while (true) {
     rateLimitTracker.printStatsIfNeeded();
@@ -217,8 +185,8 @@ Future<Never> logic(
         db,
         centralCommand,
         caches,
-        queue,
         waiter,
+        updater,
         shipFilter: shipFilter,
         loopCount: config.loopCount,
       );