Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { AreYouSureDialogComponent } from "src/app/are-you-sure-dialog/are-you-s
import saveAs from "file-saver";
import { AuthenticationService } from "src/app/services/authentication.service";
import { OperatorService } from "src/app/services/operator.service";
import { DataUpdateService } from "src/app/services/data-update.service";
import { MatButton } from "@angular/material/button";
import { MatIcon } from "@angular/material/icon";
import { MatFormField, MatLabel, MatSuffix } from "@angular/material/form-field";
Expand Down Expand Up @@ -77,6 +78,7 @@ export class RouteDetailComponent implements OnInit {
private dateAdapter = inject<DateAdapter<any>>(DateAdapter);
private dialog = inject(MatDialog);
private router = inject(Router);
private dataUpdateService = inject(DataUpdateService);

routeId: number;
route: Route;
Expand Down Expand Up @@ -180,6 +182,7 @@ export class RouteDetailComponent implements OnInit {
route.operatorIds = this.activeOperators();
}
this.apiService.updateRoute(values as Route).subscribe((_) => {
this.dataUpdateService.requestUpdate();
if (!goToInstances) {
this.goBack();
} else {
Expand Down Expand Up @@ -215,6 +218,7 @@ export class RouteDetailComponent implements OnInit {
dialogRef.afterClosed().subscribe((result: boolean) => {
if (result) {
this.apiService.deleteRoute(this.route.routeId).subscribe((_) => {
this.dataUpdateService.requestUpdate();
this.goBack();
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { MatDialog } from "@angular/material/dialog";
import { AreYouSureDialogComponent } from "src/app/are-you-sure-dialog/are-you-sure-dialog.component";
import { Moment } from "moment";
import moment from "moment";
import { DataUpdateService } from "src/app/services/data-update.service";
import { MatIconButton, MatButton } from "@angular/material/button";
import { MatIcon } from "@angular/material/icon";
import { MatCard, MatCardHeader, MatCardTitle, MatCardContent } from "@angular/material/card";
Expand Down Expand Up @@ -54,6 +55,7 @@ export class WizzardStep2Component implements OnInit {
private translateService = inject(TranslateService);
private dialog = inject(MatDialog);
private router = inject(Router);
private dataUpdateService = inject(DataUpdateService);

id: string;
data: OSMDataLine;
Expand Down Expand Up @@ -141,6 +143,7 @@ export class WizzardStep2Component implements OnInit {

this.apiService.importerAddRoute(this.data).subscribe(
(route) => {
this.dataUpdateService.requestUpdate();
// If this comes from Träwelling, navigate to route edit with trip data pre-populated
if (this.fromTraewelling && this.trawellingTripData) {
this.router.navigate(["/", "admin", "routes", route.routeId], {
Expand Down
8 changes: 8 additions & 0 deletions OV_DB/OVDBFrontend/src/app/map/map.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,12 @@
>
<mat-icon>refresh</mat-icon>
</button>
@if (isFromCache()) {
<span class="cache-indicator">
<mat-icon class="cache-icon">cached</mat-icon>
@if (cacheAge() !== null) {
<span class="cache-age">{{ cacheAge() }}s {{ "MAP.CACHE.AGO" | translate }}</span>
}
</span>
}
</ng-template>
21 changes: 21 additions & 0 deletions OV_DB/OVDBFrontend/src/app/map/map.component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,24 @@ mat-panel-title {
#refresh {
margin-left: 1rem;
}

.cache-indicator {
display: inline-flex;
align-items: center;
gap: 4px;
margin-left: 8px;
color: rgba(0, 0, 0, 0.6);
font-size: 12px;

.cache-icon {
font-size: 16px;
width: 16px;
height: 16px;
vertical-align: middle;
}

.cache-age {
font-size: 12px;
opacity: 0.7;
}
}
46 changes: 44 additions & 2 deletions OV_DB/OVDBFrontend/src/app/map/map.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import { TranslationService } from "../services/translation.service";
import { Router, ActivatedRoute } from "@angular/router";
import { MapInstanceDialogComponent } from "../map-instance-dialog/map-instance-dialog.component";
import { switchMap } from "rxjs/operators";
import { Observable } from "rxjs";
import { Observable, of } from "rxjs";
import { MapDataDTO } from "../models/map-data.model";
import { v4 as uuidv4 } from "uuid";
import { SignalRService } from "../services/signal-r.service";
import { MapDataCacheService } from "../services/map-data-cache.service";
import { DataUpdateService } from "../services/data-update.service";
import {
NgTemplateOutlet,
NgClass,
Expand Down Expand Up @@ -60,10 +62,14 @@ export class MapComponent implements OnInit, AfterViewInit, OnDestroy {
private activatedRoute = inject(ActivatedRoute);
private signalRService = inject(SignalRService);
private cd = inject(ChangeDetectorRef);
private mapDataCacheService = inject(MapDataCacheService);
private dataUpdateService = inject(DataUpdateService);

readonly guid = input<string>(undefined);
readonly mapContainer = viewChild<HTMLElement>("mapContainer");
loading: boolean | number = false;
isFromCache = signal<boolean>(false);
cacheAge = signal<number | null>(null);
from: moment.Moment;
to: moment.Moment;
selectedRegion: number[] = [];
Expand Down Expand Up @@ -228,6 +234,11 @@ export class MapComponent implements OnInit, AfterViewInit, OnDestroy {
}
},
});

// Subscribe to data updates to clear cache
this.dataUpdateService.updateRequested$.subscribe(() => {
this.mapDataCacheService.clear();
});
Comment on lines +239 to +241
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak: The subscription to dataUpdateService.updateRequested$ is not unsubscribed when the component is destroyed. This subscription should be stored and unsubscribed in the ngOnDestroy method to prevent memory leaks.

Consider storing the subscription and cleaning it up:

private updateSubscription: Subscription;

ngOnInit() {
  // ... other code ...
  
  this.updateSubscription = this.dataUpdateService.updateRequested$.subscribe(() => {
    this.mapDataCacheService.clear();
  });
}

ngOnDestroy() {
  this.signalRService.disconnect();
  this.updateSubscription?.unsubscribe();
}

Copilot uses AI. Check for mistakes.
}

ngOnDestroy() {
Expand Down Expand Up @@ -271,8 +282,32 @@ export class MapComponent implements OnInit, AfterViewInit, OnDestroy {
this.setApplicableFilter();
}

private getCurrentCacheKey(filter: string): string {
return this.mapDataCacheService.getCacheKey(
this.guid() || '',
filter,
this.translationService.language,
this.includeLineColours,
this.limitToSelectedArea
);
}

private getRoutes(filter: string): Observable<MapDataDTO> {
//Generate a GUID
// Check cache first
const cacheKey = this.getCurrentCacheKey(filter);

const cachedData = this.mapDataCacheService.get(cacheKey);
if (cachedData) {
this.isFromCache.set(true);
this.cacheAge.set(this.mapDataCacheService.getCacheAge(cacheKey));
return of(cachedData);
}

// Not in cache, fetch from API
this.isFromCache.set(false);
this.cacheAge.set(null);

// Generate a GUID
this.requestIdentifier = uuidv4();

return this.apiService.getRoutes(
Expand All @@ -285,6 +320,12 @@ export class MapComponent implements OnInit, AfterViewInit, OnDestroy {
);
}
private showRoutes(data: MapDataDTO) {
// Cache the data if it's not from cache
if (!this.isFromCache()) {
const cacheKey = this.getCurrentCacheKey(this.getFilter());
this.mapDataCacheService.set(cacheKey, data);
Comment on lines +325 to +326
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inefficient call to getFilter() with unintended side effects. The getFilter() method not only returns the filter string but also sets this.loading = true and navigates the router. Since this code is in showRoutes() which is called after the data is received, calling getFilter() here:

  1. Sets loading = true unnecessarily (it should be false at this point)
  2. Potentially triggers unnecessary router navigation
  3. Recomputes the filter that was already passed to getRoutes(filter)

Instead, the filter string should be stored as an instance variable when getRoutes() is called and reused here:

private currentFilter: string = '';

private getRoutes(filter: string): Observable<MapDataDTO> {
  this.currentFilter = filter;
  // ... rest of the code
}

private showRoutes(data: MapDataDTO) {
  if (!this.isFromCache()) {
    const cacheKey = this.getCurrentCacheKey(this.currentFilter);
    this.mapDataCacheService.set(cacheKey, data);
  }
  // ... rest of the code
}

Copilot uses AI. Check for mistakes.
}

const parent = this;
const track = geoJSON(data.routes, {
style: (feature) => {
Expand Down Expand Up @@ -566,6 +607,7 @@ export class MapComponent implements OnInit, AfterViewInit, OnDestroy {
}

refresh() {
this.mapDataCacheService.clear();
this.getRoutes$.next(this.getFilter());
}

Expand Down
102 changes: 102 additions & 0 deletions OV_DB/OVDBFrontend/src/app/services/map-data-cache.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { TestBed } from '@angular/core/testing';
import { MapDataCacheService } from './map-data-cache.service';
import { MapDataDTO } from '../models/map-data.model';

describe('MapDataCacheService', () => {
let service: MapDataCacheService;

beforeEach(() => {
TestBed.configureTestingModule({});
service = TestBed.inject(MapDataCacheService);
});

it('should be created', () => {
expect(service).toBeTruthy();
});

it('should generate unique cache keys', () => {
const key1 = service.getCacheKey('guid1', 'filter1', 'en', true, false);
const key2 = service.getCacheKey('guid1', 'filter1', 'en', false, false);
const key3 = service.getCacheKey('guid1', 'filter1', 'nl', true, false);

expect(key1).not.toEqual(key2);
expect(key1).not.toEqual(key3);
expect(key2).not.toEqual(key3);
});

it('should store and retrieve cached data', () => {
const mockData: MapDataDTO = {
routes: { type: 'FeatureCollection', features: [] },
area: null
};
const key = service.getCacheKey('guid1', 'filter1', 'en', true, false);

service.set(key, mockData);
const retrieved = service.get(key);

expect(retrieved).toEqual(mockData);
});

it('should return null for non-existent cache key', () => {
const retrieved = service.get('non-existent-key');
expect(retrieved).toBeNull();
});

it('should clear all cache', () => {
const mockData: MapDataDTO = {
routes: { type: 'FeatureCollection', features: [] },
area: null
};
const key1 = service.getCacheKey('guid1', 'filter1', 'en', true, false);
const key2 = service.getCacheKey('guid2', 'filter2', 'nl', false, true);

service.set(key1, mockData);
service.set(key2, mockData);

service.clear();

expect(service.get(key1)).toBeNull();
expect(service.get(key2)).toBeNull();
});

it('should return cache age in seconds', () => {
const mockData: MapDataDTO = {
routes: { type: 'FeatureCollection', features: [] },
area: null
};
const key = service.getCacheKey('guid1', 'filter1', 'en', true, false);

service.set(key, mockData);
const age = service.getCacheAge(key);

expect(age).toBeDefined();
expect(age).toBeGreaterThanOrEqual(0);
expect(age).toBeLessThan(2); // Should be less than 2 seconds old
});

it('should return null for cache age of non-existent key', () => {
const age = service.getCacheAge('non-existent-key');
expect(age).toBeNull();
});

it('should expire cache after 5 minutes', () => {
const mockData: MapDataDTO = {
routes: { type: 'FeatureCollection', features: [] },
area: null
};
const key = service.getCacheKey('guid1', 'filter1', 'en', true, false);

// Store data in cache
service.set(key, mockData);

// Verify data is in cache
expect(service.get(key)).toEqual(mockData);

// Mock Date.now to return time 5 minutes and 1 second in the future
spyOn(Date, 'now').and.returnValue(Date.now() + (5 * 60 * 1000 + 1000));
Comment on lines +90 to +96
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache expiration test has a timing issue. The Date.now() call in the spyOn setup captures the current time at line 96, but the cache entry was created earlier at line 90 with a potentially different timestamp. This could cause the test to be flaky if there's any significant delay between these operations.

Consider storing the timestamp when the cache is set and using that for the calculation:

// Store data in cache
const cacheTime = Date.now();
service.set(key, mockData);

// Verify data is in cache
expect(service.get(key)).toEqual(mockData);

// Mock Date.now to return time 5 minutes and 1 second after cache was set
spyOn(Date, 'now').and.returnValue(cacheTime + (5 * 60 * 1000 + 1000));
Suggested change
service.set(key, mockData);
// Verify data is in cache
expect(service.get(key)).toEqual(mockData);
// Mock Date.now to return time 5 minutes and 1 second in the future
spyOn(Date, 'now').and.returnValue(Date.now() + (5 * 60 * 1000 + 1000));
const cacheTime = Date.now();
service.set(key, mockData);
// Verify data is in cache
expect(service.get(key)).toEqual(mockData);
// Mock Date.now to return time 5 minutes and 1 second after cache was set
spyOn(Date, 'now').and.returnValue(cacheTime + (5 * 60 * 1000 + 1000));

Copilot uses AI. Check for mistakes.

// Cache should be expired now
const retrieved = service.get(key);
expect(retrieved).toBeNull();
});
});
59 changes: 59 additions & 0 deletions OV_DB/OVDBFrontend/src/app/services/map-data-cache.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { Injectable } from '@angular/core';
import { MapDataDTO } from '../models/map-data.model';

interface CachedMapData {
data: MapDataDTO;
timestamp: number;
}

@Injectable({
providedIn: 'root'
})
export class MapDataCacheService {
private cache = new Map<string, CachedMapData>();
private readonly CACHE_DURATION = 5 * 60 * 1000; // 5 minutes

getCacheKey(
guid: string,
filter: string,
language: string,
includeLineColours: boolean,
limitToSelectedArea: boolean
): string {
return `${guid}|${filter}|${language}|${includeLineColours}|${limitToSelectedArea}`;
}

get(key: string): MapDataDTO | null {
const cached = this.cache.get(key);

if (!cached) {
return null;
}

// Check if expired
if (Date.now() - cached.timestamp > this.CACHE_DURATION) {
this.cache.delete(key);
return null;
}

return cached.data;
}

set(key: string, data: MapDataDTO): void {
this.cache.set(key, {
data,
timestamp: Date.now()
});
}

clear(): void {
this.cache.clear();
}

// Get cache age in seconds for a key (for UI display)
getCacheAge(key: string): number | null {
const cached = this.cache.get(key);
if (!cached) return null;
return Math.floor((Date.now() - cached.timestamp) / 1000);
}
}
1 change: 1 addition & 0 deletions OV_DB/OVDBFrontend/src/assets/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@
"MAP.POPUP.REMARK": "Remark",
"MAP.POPUP.LINENUMBER": "Line number",
"MAP.POPUP.OPERATINGCOMPANY": "Operating company",
"MAP.CACHE.AGO": "ago",
"ADDMAP.SHOWROUTEINFO": "Show route information on map",
"ADDMAP.SHOWROUTEOUTLINE": "Highlight route on selection",
"ROUTESLIST.SELECT": "Select",
Expand Down
1 change: 1 addition & 0 deletions OV_DB/OVDBFrontend/src/assets/i18n/nl.json
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@
"MAP.POPUP.REMARK": "Opmerking",
"MAP.POPUP.LINENUMBER": "Lijnnummer",
"MAP.POPUP.OPERATINGCOMPANY": "Bedienend bedrijf",
"MAP.CACHE.AGO": "geleden",
"ADDMAP.SHOWROUTEINFO": "Laat route informatie zien op de kaart",
"ADDMAP.SHOWROUTEOUTLINE": "Benadruk route bij selectie",
"ROUTESLIST.SELECT": "Selecteer",
Expand Down
Loading