From 396687603b564412d5e9489578b45b13d358d5ec Mon Sep 17 00:00:00 2001 From: anandhkb Date: Sat, 28 Feb 2026 20:19:56 -0800 Subject: [PATCH] Fix #903: Validate unique vehicle_ids in VRP fleet data - Add uniqueness check in validate_fleet_data() for vehicle_ids - Return clear error when duplicates detected - Update FleetData.vehicle_ids docstring - Add unit tests: duplicate rejection, unique acceptance, edge cases --- .../cuopt_server/tests/test_set_fleet_data.py | 144 ++++++++++++++++++ .../utils/routing/data_definition.py | 5 +- .../utils/routing/validation_fleet_data.py | 5 + 3 files changed, 153 insertions(+), 1 deletion(-) diff --git a/python/cuopt_server/cuopt_server/tests/test_set_fleet_data.py b/python/cuopt_server/cuopt_server/tests/test_set_fleet_data.py index db2402eeba..51047c3485 100644 --- a/python/cuopt_server/cuopt_server/tests/test_set_fleet_data.py +++ b/python/cuopt_server/cuopt_server/tests/test_set_fleet_data.py @@ -5,6 +5,7 @@ from cuopt_server.tests.utils.utils import cuoptproc # noqa from cuopt_server.tests.utils.utils import RequestClient +from cuopt_server.utils.routing.validation_fleet_data import validate_fleet_data client = RequestClient() @@ -62,6 +63,117 @@ # FLEET DATA TESTING +# Test validate_fleet_data rejects duplicate vehicle_ids (no server required) +def test_validate_fleet_data_duplicate_vehicle_ids(): + vehicle_locations = [[0, 0], [0, 0], [0, 0]] + vehicle_ids_dup = ["Truck 1", "Truck 1", "Truck 1"] + + is_valid, msg = validate_fleet_data( + vehicle_ids=vehicle_ids_dup, + vehicle_locations=vehicle_locations, + capacities=None, + vehicle_time_windows=None, + vehicle_breaks=None, + vehicle_break_time_windows=None, + vehicle_break_durations=None, + vehicle_break_locations=None, + vehicle_types=None, + vehicle_types_dict={}, + vehicle_order_match=None, + skip_first_trips=None, + drop_return_trips=None, + min_vehicles=None, + vehicle_max_costs=None, + vehicle_max_times=None, + vehicle_fixed_costs=None, + ) + assert is_valid is False + assert "unique" in msg.lower() and "duplicate" in msg.lower() + + +# Test validate_fleet_data accepts unique vehicle_ids (no server required) +def test_validate_fleet_data_unique_vehicle_ids(): + vehicle_locations = [[0, 0], [0, 0]] + vehicle_ids_unique = ["Truck 1", "Truck 2"] + + is_valid, msg = validate_fleet_data( + vehicle_ids=vehicle_ids_unique, + vehicle_locations=vehicle_locations, + capacities=None, + vehicle_time_windows=None, + vehicle_breaks=None, + vehicle_break_time_windows=None, + vehicle_break_durations=None, + vehicle_break_locations=None, + vehicle_types=None, + vehicle_types_dict={}, + vehicle_order_match=None, + skip_first_trips=None, + drop_return_trips=None, + min_vehicles=None, + vehicle_max_costs=None, + vehicle_max_times=None, + vehicle_fixed_costs=None, + ) + assert is_valid is True + assert msg == "Valid Fleet Data" + + +# Test validate_fleet_data with vehicle_ids=None passes (no server required) +def test_validate_fleet_data_vehicle_ids_none(): + vehicle_locations = [[0, 0], [0, 0]] + + is_valid, msg = validate_fleet_data( + vehicle_ids=None, + vehicle_locations=vehicle_locations, + capacities=None, + vehicle_time_windows=None, + vehicle_breaks=None, + vehicle_break_time_windows=None, + vehicle_break_durations=None, + vehicle_break_locations=None, + vehicle_types=None, + vehicle_types_dict={}, + vehicle_order_match=None, + skip_first_trips=None, + drop_return_trips=None, + min_vehicles=None, + vehicle_max_costs=None, + vehicle_max_times=None, + vehicle_fixed_costs=None, + ) + assert is_valid is True + assert msg == "Valid Fleet Data" + + +# Test validate_fleet_data with single vehicle (no server required) +def test_validate_fleet_data_single_vehicle(): + vehicle_locations = [[0, 0]] + vehicle_ids_single = ["Truck 1"] + + is_valid, msg = validate_fleet_data( + vehicle_ids=vehicle_ids_single, + vehicle_locations=vehicle_locations, + capacities=None, + vehicle_time_windows=None, + vehicle_breaks=None, + vehicle_break_time_windows=None, + vehicle_break_durations=None, + vehicle_break_locations=None, + vehicle_types=None, + vehicle_types_dict={}, + vehicle_order_match=None, + skip_first_trips=None, + drop_return_trips=None, + min_vehicles=None, + vehicle_max_costs=None, + vehicle_max_times=None, + vehicle_fixed_costs=None, + ) + assert is_valid is True + assert msg == "Valid Fleet Data" + + # Test validation error when multiple cost matrices set without vehicle types def test_invalid_vehicle_types(cuoptproc): # noqa matrix_data = { @@ -101,6 +213,38 @@ def test_valid_full_set_fleet_data(cuoptproc): # noqa assert response_set.status_code == 200 +# Testing duplicate vehicle_ids rejected (issue #903) +def test_duplicate_vehicle_ids_set_fleet_data(cuoptproc): # noqa + test_data = copy.deepcopy(valid_data) + test_data["fleet_data"]["vehicle_ids"] = [ + "veh-1", + "veh-2", + "veh-1", + "veh-4", + ] + + response_set = client.post("/cuopt/request", json=test_data) + assert response_set.status_code == 400 + assert response_set.json() == { + "error": "vehicle_ids must be unique; duplicates are not allowed", + "error_result": True, + } + + +# Testing valid with unique vehicle_ids +def test_valid_unique_vehicle_ids_set_fleet_data(cuoptproc): # noqa + test_data = copy.deepcopy(valid_data) + test_data["fleet_data"]["vehicle_ids"] = [ + "veh-1", + "veh-2", + "veh-3", + "veh-4", + ] + + response_set = client.post("/cuopt/request", json=test_data) + assert response_set.status_code == 200 + + # Testing valid with minimal required parameters def test_valid_minimal_set_fleet_data(cuoptproc): # noqa test_data = copy.deepcopy(valid_data) diff --git a/python/cuopt_server/cuopt_server/utils/routing/data_definition.py b/python/cuopt_server/cuopt_server/utils/routing/data_definition.py index 4a0abda553..18b9d2f22e 100644 --- a/python/cuopt_server/cuopt_server/utils/routing/data_definition.py +++ b/python/cuopt_server/cuopt_server/utils/routing/data_definition.py @@ -232,7 +232,10 @@ class FleetData(StrictModel): vehicle_ids: Optional[List[str]] = Field( default=None, examples=[["veh-1", "veh-2"]], - description=("List of the vehicle ids or names provided as a string."), + description=( + "List of the vehicle ids or names provided as a string. " + "Must be unique; duplicates are not allowed." + ), ) capacities: Optional[List[List[int]]] = Field( default=None, diff --git a/python/cuopt_server/cuopt_server/utils/routing/validation_fleet_data.py b/python/cuopt_server/cuopt_server/utils/routing/validation_fleet_data.py index 5a505e8ef6..1b2566751a 100644 --- a/python/cuopt_server/cuopt_server/utils/routing/validation_fleet_data.py +++ b/python/cuopt_server/cuopt_server/utils/routing/validation_fleet_data.py @@ -85,6 +85,11 @@ def validate_fleet_data( if vehicle_ids is not None: fleet_length_check_array.append(len(vehicle_ids)) + if len(vehicle_ids) != len(set(vehicle_ids)): + return ( + False, + "vehicle_ids must be unique; duplicates are not allowed", + ) if capacities is not None: fleet_length_check_array.append(len(capacities[0]))