Skip to content

Commit 73a1b0b

Browse files
testowertestower
committed
Fix update logic after regression
1 parent 17c7d60 commit 73a1b0b

File tree

4 files changed

+126
-46
lines changed

4 files changed

+126
-46
lines changed

‎src/main/java/org/entur/lamassu/leader/entityupdater/StationsUpdater.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -248,19 +248,20 @@ private void processDeltaUpdate(
248248
currentStation,
249249
context.feedProvider
250250
);
251-
var newSpatialIndexId = spatialIndexService.createStationIndexId(
252-
mappedStation,
253-
context.feedProvider
254-
);
255251

256252
// Merge the mapped station into the current station
257253
stationMergeMapper.updateStation(currentStation, mappedStation);
258254
context.addedAndUpdatedStations.put(currentStation.getId(), currentStation);
259255

256+
var newSpatialIndexId = spatialIndexService.createStationIndexId(
257+
currentStation,
258+
context.feedProvider
259+
);
260+
260261
if (!oldSpatialIndexId.equals(newSpatialIndexId)) {
261262
context.spatialIndexIdsToRemove.add(oldSpatialIndexId);
262-
context.spatialIndexUpdateMap.put(newSpatialIndexId, currentStation);
263263
}
264+
context.spatialIndexUpdateMap.put(newSpatialIndexId, currentStation);
264265
} else {
265266
logger.warn(
266267
"Station {} marked for update but not found in cache",

‎src/main/java/org/entur/lamassu/leader/entityupdater/VehiclesUpdater.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -189,18 +189,19 @@ private void processDeltaUpdate(
189189
currentVehicle,
190190
context.feedProvider
191191
);
192-
var newSpatialIndexId = spatialIndexService.createVehicleIndexId(
193-
mappedVehicle,
194-
context.feedProvider
195-
);
196192

197193
vehicleMergeMapper.updateVehicle(currentVehicle, mappedVehicle);
198194
context.addedAndUpdatedVehicles.put(currentVehicle.getId(), currentVehicle);
199195

196+
var newSpatialIndexId = spatialIndexService.createVehicleIndexId(
197+
currentVehicle,
198+
context.feedProvider
199+
);
200+
200201
if (!oldSpatialIndexId.equals(newSpatialIndexId)) {
201202
context.spatialIndexIdsToRemove.add(oldSpatialIndexId);
202-
context.spatialIndexUpdateMap.put(newSpatialIndexId, currentVehicle);
203203
}
204+
context.spatialIndexUpdateMap.put(newSpatialIndexId, currentVehicle);
204205
} else {
205206
logger.warn(
206207
"Vehicle {} marked for update but not found in cache",

‎src/test/java/org/entur/lamassu/leader/entityupdater/StationsUpdaterTest.java

+106-18
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@
99
import static org.mockito.Mockito.verify;
1010
import static org.mockito.Mockito.when;
1111

12+
import java.util.ArrayList;
1213
import java.util.Date;
1314
import java.util.List;
1415
import java.util.Set;
1516
import java.util.concurrent.TimeUnit;
1617
import org.entur.lamassu.cache.EntityCache;
1718
import org.entur.lamassu.cache.StationSpatialIndex;
18-
import org.entur.lamassu.cache.StationSpatialIndexId;
1919
import org.entur.lamassu.delta.DeltaType;
2020
import org.entur.lamassu.delta.GBFSEntityDelta;
2121
import org.entur.lamassu.delta.GBFSFileDelta;
@@ -285,38 +285,60 @@ void shouldHandleStationUpdateWithSpatialIndexChange() {
285285

286286
var stationId = "station-1";
287287
var bikeTypeId = "bike";
288+
var scooterTypeId = "scooter";
288289

289290
var bikeType = new VehicleType();
290291
bikeType.setId(bikeTypeId);
291292
bikeType.setFormFactor(FormFactor.BICYCLE);
292293
bikeType.setPropulsionType(PropulsionType.HUMAN);
293294

295+
var scooterType = new VehicleType();
296+
scooterType.setId(scooterTypeId);
297+
scooterType.setFormFactor(FormFactor.SCOOTER);
298+
scooterType.setPropulsionType(PropulsionType.ELECTRIC);
299+
294300
var currentStation = new Station();
295301
currentStation.setId(stationId);
296302
currentStation.setLat(59.9);
297303
currentStation.setLon(10.7);
298304
var oldAvailability = new VehicleTypeAvailability();
299305
oldAvailability.setVehicleTypeId(bikeTypeId);
300306
oldAvailability.setCount(5);
301-
currentStation.setVehicleTypesAvailable(List.of(oldAvailability));
307+
currentStation.setVehicleTypesAvailable(new ArrayList<>(List.of(oldAvailability)));
302308

303309
var stationInfo = new GBFSStation();
304310
stationInfo.setStationId(stationId);
305-
stationInfo.setName(
306-
List.of(new GBFSName().withLanguage("en").withText("Test Station"))
307-
);
308311
stationInfo.setLat(59.9);
309312
stationInfo.setLon(10.7);
313+
stationInfo.setName(
314+
new ArrayList<>(List.of(new GBFSName().withLanguage("en").withText("Test Station")))
315+
);
316+
stationInfo.setVehicleTypesCapacity(
317+
new ArrayList<>(
318+
List.of(
319+
new org.mobilitydata.gbfs.v3_0.station_information.GBFSVehicleTypesCapacity()
320+
.withVehicleTypeIds(new ArrayList<>(List.of(scooterTypeId)))
321+
.withCount(3)
322+
)
323+
)
324+
);
310325

311326
var stationStatus = new org.mobilitydata.gbfs.v3_0.station_status.GBFSStation();
312327
stationStatus.setStationId(stationId);
313-
// No vehicles available anymore
314-
stationStatus.setNumVehiclesAvailable(0);
315328
stationStatus.setNumDocksAvailable(10);
316329
stationStatus.setIsInstalled(true);
317330
stationStatus.setIsRenting(true);
318331
stationStatus.setIsReturning(true);
319332
stationStatus.setLastReported(new Date());
333+
stationStatus.setVehicleTypesAvailable(
334+
new ArrayList<>(
335+
List.of(
336+
new org.mobilitydata.gbfs.v3_0.station_status.GBFSVehicleTypesAvailable()
337+
.withVehicleTypeId(scooterTypeId)
338+
.withCount(3)
339+
)
340+
)
341+
);
320342

321343
var stationInformationFeed = new GBFSStationInformation();
322344
var data = new GBFSData();
@@ -326,6 +348,7 @@ void shouldHandleStationUpdateWithSpatialIndexChange() {
326348
// Mock behavior
327349
when(stationCache.get(stationId)).thenReturn(currentStation);
328350
when(vehicleTypeCache.getAll(Set.of(bikeTypeId))).thenReturn(List.of(bikeType));
351+
when(vehicleTypeCache.getAll(Set.of(scooterTypeId))).thenReturn(List.of(scooterType));
329352

330353
var delta = new GBFSFileDelta<>(
331354
1000L,
@@ -399,19 +422,84 @@ void shouldRemoveExistingStationsWhenBaseIsNull() {
399422
if (spatialIds.size() != 2) return false;
400423
return spatialIds
401424
.stream()
402-
.allMatch(id -> {
403-
var spatialId = (StationSpatialIndexId) id;
404-
return (
405-
(
406-
spatialId.getId().equals("station-1") ||
407-
spatialId.getId().equals("station-2")
408-
) &&
409-
spatialId.getSystemId().equals("system-1") &&
410-
spatialId.getCodespace().equals("codespace-1")
411-
);
412-
});
425+
.allMatch(id ->
426+
(
427+
(id.getId().equals("station-1") || id.getId().equals("station-2")) &&
428+
id.getSystemId().equals("system-1") &&
429+
id.getCodespace().equals("codespace-1")
430+
)
431+
);
413432
})
414433
);
415434
verify(stationCache, never()).removeAll(Set.of("station-3")); // Should not remove stations from other systems
416435
}
436+
437+
@Test
438+
void shouldPreservePropertiesWhenGeneratingSpatialIndexId() {
439+
// Given
440+
var feedProvider = new FeedProvider();
441+
feedProvider.setSystemId("test-system");
442+
feedProvider.setCodespace("test");
443+
feedProvider.setOperatorId("test-operator");
444+
feedProvider.setLanguage("en");
445+
446+
var stationId = "station-1";
447+
var bikeTypeId = "bike";
448+
449+
var bikeType = new VehicleType();
450+
bikeType.setId(bikeTypeId);
451+
bikeType.setFormFactor(FormFactor.BICYCLE);
452+
bikeType.setPropulsionType(PropulsionType.HUMAN);
453+
454+
var currentStation = new Station();
455+
currentStation.setId(stationId);
456+
currentStation.setLat(59.9);
457+
currentStation.setLon(10.7);
458+
var availability = new VehicleTypeAvailability();
459+
availability.setVehicleTypeId(bikeTypeId);
460+
availability.setCount(5);
461+
currentStation.setVehicleTypesAvailable(new ArrayList<>(List.of(availability)));
462+
463+
var stationInfo = new GBFSStation();
464+
stationInfo.setStationId(stationId);
465+
stationInfo.setLat(59.9);
466+
stationInfo.setLon(10.7);
467+
stationInfo.setName(
468+
new ArrayList<>(List.of(new GBFSName().withLanguage("en").withText("Test Station")))
469+
);
470+
// Note: not setting vehicle types in GBFS station
471+
472+
var stationStatus = new org.mobilitydata.gbfs.v3_0.station_status.GBFSStation();
473+
stationStatus.setStationId(stationId);
474+
stationStatus.setNumDocksAvailable(10);
475+
stationStatus.setIsInstalled(true);
476+
stationStatus.setIsRenting(true);
477+
stationStatus.setIsReturning(true);
478+
stationStatus.setLastReported(new Date());
479+
480+
var stationInformationFeed = new GBFSStationInformation();
481+
var data = new GBFSData();
482+
data.setStations(List.of(stationInfo));
483+
stationInformationFeed.setData(data);
484+
485+
// Mock behavior
486+
when(stationCache.get(stationId)).thenReturn(currentStation);
487+
when(vehicleTypeCache.getAll(Set.of(bikeTypeId))).thenReturn(List.of(bikeType));
488+
489+
var delta = new GBFSFileDelta<org.mobilitydata.gbfs.v3_0.station_status.GBFSStation>(
490+
1000L,
491+
2000L,
492+
"station_status",
493+
List.of(new GBFSEntityDelta<>(stationId, DeltaType.UPDATE, stationStatus))
494+
);
495+
496+
// When
497+
stationsUpdater.update(feedProvider, delta, stationInformationFeed);
498+
499+
// Then
500+
verify(spatialIndex).addAll(any());
501+
verify(stationCache).updateAll(any());
502+
verify(spatialIndex, never()).removeAll(anySet());
503+
verify(stationCache, never()).removeAll(anySet());
504+
}
417505
}

‎src/test/java/org/entur/lamassu/leader/entityupdater/VehiclesUpdaterTest.java

+8-18
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,11 @@ void shouldHandleVehicleUpdate() {
122122
List.of(new GBFSEntityDelta<>(vehicleId, DeltaType.UPDATE, gbfsVehicle))
123123
);
124124

125-
// Calculate expected spatial index ID for verification
126-
var oldSpatialIndexId = spatialIndexIdGeneratorService.createVehicleIndexId(
127-
currentVehicle,
128-
feedProvider
129-
);
130-
131125
// When
132126
vehiclesUpdater.update(feedProvider, delta);
133127

134128
// Then
135-
verify(spatialIndex).removeAll(Set.of(oldSpatialIndexId));
129+
verify(spatialIndex, never()).removeAll(any());
136130
verify(spatialIndex).addAll(any());
137131
verify(vehicleCache).updateAll(any());
138132
verify(vehicleCache, never()).removeAll(anySet());
@@ -320,18 +314,14 @@ void shouldRemoveExistingVehiclesWhenBaseIsNull() {
320314
return spatialIds
321315
.stream()
322316
.allMatch(id -> {
323-
var spatialId = (VehicleSpatialIndexId) id;
324317
return (
325-
(
326-
spatialId.getId().equals("vehicle-1") ||
327-
spatialId.getId().equals("vehicle-2")
328-
) &&
329-
spatialId.getSystemId().equals("system-1") &&
330-
spatialId.getCodespace().equals("codespace-1") &&
331-
spatialId.getFormFactor() == FormFactor.BICYCLE &&
332-
spatialId.getPropulsionType() == PropulsionType.HUMAN &&
333-
!spatialId.getReserved() &&
334-
!spatialId.getDisabled()
318+
(id.getId().equals("vehicle-1") || id.getId().equals("vehicle-2")) &&
319+
id.getSystemId().equals("system-1") &&
320+
id.getCodespace().equals("codespace-1") &&
321+
id.getFormFactor() == FormFactor.BICYCLE &&
322+
id.getPropulsionType() == PropulsionType.HUMAN &&
323+
!id.getReserved() &&
324+
!id.getDisabled()
335325
);
336326
});
337327
})

0 commit comments

Comments
 (0)