update timed loader to use class composition

src/entities/InterchangeSystem.ts

@@ -1,6 +1,5 @@
-import { ShuttleRepositoryLoader } from "../loaders/ShuttleRepositoryLoader";
 import { ETANotificationScheduler } from "../notifications/schedulers/ETANotificationScheduler";
-import { TimedApiBasedShuttleRepositoryLoader } from "../loaders/TimedApiBasedShuttleRepositoryLoader";
+import { TimedApiBasedRepositoryLoader } from "../loaders/TimedApiBasedRepositoryLoader";
 import { UnoptimizedInMemoryShuttleRepository } from "../repositories/UnoptimizedInMemoryShuttleRepository";
 import { RedisNotificationRepository } from "../repositories/RedisNotificationRepository";
 import { NotificationRepository } from "../repositories/NotificationRepository";
@@ -27,7 +26,7 @@ export class InterchangeSystem {
   constructor(
     public name: string,
     public id: string,
-    public shuttleDataLoader: ShuttleRepositoryLoader,
+    public shuttleTimedDataLoader: TimedApiBasedRepositoryLoader,
     public shuttleRepository: ShuttleGetterSetterRepository,
     public notificationScheduler: ETANotificationScheduler,
     public notificationRepository: NotificationRepository,
@@ -43,12 +42,15 @@ export class InterchangeSystem {
     args: InterchangeSystemBuilderArguments,
   ) {
     const shuttleRepository = new UnoptimizedInMemoryShuttleRepository();
-    const shuttleDataLoader = new TimedApiBasedShuttleRepositoryLoader(
+    const shuttleDataLoader = new ApiBasedShuttleRepositoryLoader(
       args.passioSystemId,
       args.id,
       shuttleRepository
     );
-    await shuttleDataLoader.start();
+    const timedShuttleDataLoader = new TimedApiBasedRepositoryLoader(
+      shuttleDataLoader,
+    );
+    await timedShuttleDataLoader.start();
 
     const notificationRepository = new RedisNotificationRepository();
     await notificationRepository.connect();
@@ -62,7 +64,7 @@ export class InterchangeSystem {
     return new InterchangeSystem(
       args.name,
       args.id,
-      shuttleDataLoader,
+      timedShuttleDataLoader,
       shuttleRepository,
       notificationScheduler,
       notificationRepository,
@@ -84,6 +86,11 @@ export class InterchangeSystem {
       args.id,
       shuttleRepository
     );
+    // Note that this loader should not be started,
+    // so the test data doesn't get overwritten
+    const timedShuttleLoader = new TimedApiBasedRepositoryLoader(
+      shuttleDataLoader,
+    );
 
     const notificationRepository = new InMemoryNotificationRepository();
     const notificationScheduler = new ETANotificationScheduler(
@@ -96,7 +103,7 @@ export class InterchangeSystem {
     return new InterchangeSystem(
       args.name,
       args.id,
-      shuttleDataLoader,
+      timedShuttleLoader,
       shuttleRepository,
       notificationScheduler,
       notificationRepository,

src/loaders/TimedApiBasedRepositoryLoader.ts

@@ -1,5 +1,4 @@
-import { ShuttleGetterSetterRepository } from "../repositories/ShuttleGetterSetterRepository";
-import { ApiBasedShuttleRepositoryLoader } from "./ApiBasedShuttleRepositoryLoader";
+import { RepositoryLoader } from "./RepositoryLoader";
 
 // Ideas to break this into smaller pieces in the future:
 // Have one repository data loader running for each supported system
@@ -15,18 +14,15 @@ import { ApiBasedShuttleRepositoryLoader } from "./ApiBasedShuttleRepositoryLoad
 // - OrderedStops: reload every few minutes
 // - Systems: reload once a day
 
-export class TimedApiBasedShuttleRepositoryLoader extends ApiBasedShuttleRepositoryLoader {
+export class TimedApiBasedRepositoryLoader {
   private shouldBeRunning: boolean = false;
   private timer: any;
 
   readonly timeout = 10000;
 
   constructor(
-    public passioSystemId: string,
-    public systemIdForConstructedData: string,
-    repository: ShuttleGetterSetterRepository,
+    public loader: RepositoryLoader,
   ) {
-    super(passioSystemId, systemIdForConstructedData, repository);
     this.startFetchDataAndUpdate = this.startFetchDataAndUpdate.bind(this);
   }
 
@@ -48,14 +44,7 @@ export class TimedApiBasedShuttleRepositoryLoader extends ApiBasedShuttleReposit
     if (!this.shouldBeRunning) return;
 
     try {
-      await this.fetchAndUpdateRouteDataForSystem();
-      await this.fetchAndUpdateStopAndPolylineDataForRoutesInSystem();
-      await this.fetchAndUpdateShuttleDataForSystem();
-
-      // Because ETA method doesn't support pruning yet,
-      // add a call to the clear method here
-      await this.repository.clearEtaData();
-      await this.fetchAndUpdateEtaDataForExistingStopsForSystem();
+      await this.loader.fetchAndUpdateAll();
     } catch (e) {
       console.error(e);
     }

test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts

@@ -1,10 +1,11 @@
 import { afterEach, beforeAll, beforeEach, describe, expect, it, jest } from "@jest/globals";
-import { TimedApiBasedShuttleRepositoryLoader } from "../../src/loaders/TimedApiBasedShuttleRepositoryLoader";
+import { TimedApiBasedRepositoryLoader } from "../../src/loaders/TimedApiBasedRepositoryLoader";
 import { resetGlobalFetchMockJson } from "../testHelpers/fetchMockHelpers";
 import { UnoptimizedInMemoryShuttleRepository } from "../../src/repositories/UnoptimizedInMemoryShuttleRepository";
+import { ApiBasedShuttleRepositoryLoader } from "../../src/loaders/ApiBasedShuttleRepositoryLoader";
 
 describe("TimedApiBasedRepositoryLoader", () => {
-  let loader: TimedApiBasedShuttleRepositoryLoader;
+  let timedLoader: TimedApiBasedRepositoryLoader;
   let spies: any;
 
   beforeAll(() => {
@@ -15,17 +16,17 @@ describe("TimedApiBasedRepositoryLoader", () => {
   beforeEach(() => {
     resetGlobalFetchMockJson();
 
-    loader = new TimedApiBasedShuttleRepositoryLoader(
+    const mockLoader = new ApiBasedShuttleRepositoryLoader(
       "1",
       "1",
-      new UnoptimizedInMemoryShuttleRepository()
+      new UnoptimizedInMemoryShuttleRepository(),
+    );
+    timedLoader = new TimedApiBasedRepositoryLoader(
+      mockLoader,
     );
 
     spies = {
-      fetchAndUpdateRouteDataForSystem: jest.spyOn(loader, 'fetchAndUpdateRouteDataForSystem'),
-      fetchAndUpdateStopAndPolylineDataForRoutesInSystem: jest.spyOn(loader, 'fetchAndUpdateStopAndPolylineDataForRoutesInSystem'),
-      fetchAndUpdateShuttleDataForSystem: jest.spyOn(loader, 'fetchAndUpdateShuttleDataForSystem'),
-      fetchAndUpdateEtaDataForExistingStopsForSystem: jest.spyOn(loader, 'fetchAndUpdateEtaDataForExistingStopsForSystem')
+      fetchAndUpdateAll: jest.spyOn(mockLoader, 'fetchAndUpdateAll'),
     };
 
     Object.values(spies).forEach((spy: any) => {
@@ -40,20 +41,20 @@ describe("TimedApiBasedRepositoryLoader", () => {
 
   describe("start", () => {
     it("should update internal state, call data fetching methods, and start a timer", async () => {
-      await loader.start();
-      expect(loader["shouldBeRunning"]).toBe(true);
+      await timedLoader.start();
+      expect(timedLoader["shouldBeRunning"]).toBe(true);
 
       Object.values(spies).forEach((spy: any) => {
         expect(spy).toHaveBeenCalled();
       });
 
-      expect(setTimeout).toHaveBeenCalledWith(expect.any(Function), loader.timeout);
-      expect(loader.timeout).not.toBeUndefined();
+      expect(setTimeout).toHaveBeenCalledWith(expect.any(Function), timedLoader.timeout);
+      expect(timedLoader.timeout).not.toBeUndefined();
     });
 
     it("does nothing if timer is already running", async () => {
-      await loader.start();
-      await loader.start();
+      await timedLoader.start();
+      await timedLoader.start();
 
       Object.values(spies).forEach((spy: any) => {
         expect(spy).toHaveBeenCalledTimes(1);
@@ -63,8 +64,8 @@ describe("TimedApiBasedRepositoryLoader", () => {
 
   describe("stop", () => {
     it("should update internal state", async () => {
-      loader.stop();
-      expect(loader['shouldBeRunning']).toBe(false);
+      timedLoader.stop();
+      expect(timedLoader['shouldBeRunning']).toBe(false);
     });
   });
 });