diff --git a/src/api.py b/src/api.py index 118f248..6bbbd48 100644 --- a/src/api.py +++ b/src/api.py @@ -86,8 +86,21 @@ async def validation_exception_handler( ): exc_str = parse_error(exc) logger.error("%s: %s", request, exc_str) + # Answer in FastAPI's standard `{"detail": [{loc, msg, type}]}` shape. + # `input` and `ctx` are dropped: `ctx` can carry raw exception objects + # that are not JSON serialisable (the reason this handler originally + # stringified the whole error), and `input` echoes the request body. + detail = [ + { + "loc": list(error.get("loc", ())), + "msg": error.get("msg", ""), + "type": error.get("type", ""), + } + for error in exc.errors() + ] return JSONResponse( - content=exc_str, status_code=status.HTTP_422_UNPROCESSABLE_ENTITY + content={"detail": detail}, + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, ) diff --git a/src/controllers/interface.py b/src/controllers/interface.py index a3a6975..2f7516a 100644 --- a/src/controllers/interface.py +++ b/src/controllers/interface.py @@ -1,5 +1,6 @@ import functools from typing import List +from pydantic import ValidationError from pymongo.errors import PyMongoError from fastapi import HTTPException, status @@ -22,6 +23,27 @@ async def wrapper(self, *args, **kwargs): ) except HTTPException as e: raise e from e + except ValidationError as e: + # pydantic's ValidationError subclasses ValueError, but a model + # failing to validate here means a persisted document or a view + # is malformed: a server fault, never client input. Keep it on + # the generic 500 path so no internal detail reaches the client. + logger.exception(f"{method.__name__}: Unexpected error {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Unexpected error", + ) from e + except ValueError as e: + # RocketPy raises ValueError for physically invalid input + # (overfilled tank, bad parachute trigger, inconsistent + # geometry). That is the client's design, not a server fault: + # return it as 422 with the message instead of a bare 500. + detail = " ".join(str(e).split()) + logger.warning(f"{method.__name__}: Invalid input {detail}") + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=detail, + ) from e except Exception as e: logger.exception(f"{method.__name__}: Unexpected error {e}") raise HTTPException( diff --git a/src/models/rocket.py b/src/models/rocket.py index c1a22cd..9a6d9b1 100644 --- a/src/models/rocket.py +++ b/src/models/rocket.py @@ -26,8 +26,11 @@ class RocketModel(ApiBaseModel): NAME: ClassVar = "rocket" METHODS: ClassVar = ("POST", "GET", "PUT", "DELETE") - # Required parameters - motor: MotorModel + # A rocket may be created before its motor exists (jarvis saves the + # airframe first). `None` maps to rocketpy's `EmptyMotor`, which a + # Rocket carries by default: the rocket persists and draws, and the + # simulate endpoints answer 422 until a real motor is attached. + motor: Optional[MotorModel] = None radius: float mass: float motor_position: float diff --git a/src/repositories/interface.py b/src/repositories/interface.py index 7887a1d..081f248 100644 --- a/src/repositories/interface.py +++ b/src/repositories/interface.py @@ -16,6 +16,7 @@ from fastapi import HTTPException, status from bson import ObjectId +from bson.errors import InvalidId from src import logger from src.secrets import Secrets @@ -61,6 +62,10 @@ async def wrapper(self, *args, **kwargs): f"{method.__name__} - Repository not initialized: {e}" ) raise + except HTTPException: + # Deliberate client-facing errors raised inside a repository + # method (e.g. malformed id → 404) must not be re-wrapped as 500. + raise except Exception as e: logger.exception( f"{method.__name__} - caught unexpected error: {e}" @@ -258,13 +263,20 @@ async def insert(self, data: dict): async def update_by_id(self, data: dict, *, data_id: str): collection = self.get_collection() assert self.model.model_validate(data) - await collection.update_one({"_id": ObjectId(data_id)}, {"$set": data}) + object_id = self._object_id_or_404(data_id) + await collection.update_one({"_id": object_id}, {"$set": data}) return self @repository_exception_handler async def find_by_id(self, *, data_id: str): collection = self.get_collection() - read_data = await collection.find_one({"_id": ObjectId(data_id)}) + try: + object_id = ObjectId(data_id) + except InvalidId: + # A malformed id can never match a document; treat it exactly + # like a missing one so the controller answers 404, not 500. + return None + read_data = await collection.find_one({"_id": object_id}) if read_data: parsed_model = self.model.model_validate(read_data) parsed_model.set_id(str(read_data["_id"])) @@ -274,9 +286,24 @@ async def find_by_id(self, *, data_id: str): @repository_exception_handler async def delete_by_id(self, *, data_id: str): collection = self.get_collection() - await collection.delete_one({"_id": ObjectId(data_id)}) + await collection.delete_one({"_id": self._object_id_or_404(data_id)}) return self + def _object_id_or_404(self, data_id: str) -> ObjectId: + """Parse a path id, answering 404 for a malformed one. + + The jarvis client was observed issuing PUT /rockets/undefined; bson + raises InvalidId for that, which the generic handler used to turn + into a 500 "Unexpected error ocurred". + """ + try: + return ObjectId(data_id) + except InvalidId as exc: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"{self.model.NAME} not found", + ) from exc + @repository_exception_handler async def find_by_query(self, query: dict): collection = self.get_collection() diff --git a/src/services/flight.py b/src/services/flight.py index b19873c..8954343 100644 --- a/src/services/flight.py +++ b/src/services/flight.py @@ -61,7 +61,9 @@ def from_flight_model(cls, flight: FlightModel) -> Self: rocketpy_env = EnvironmentService.from_env_model( flight.environment ).environment - rocketpy_rocket = RocketService.from_rocket_model(flight.rocket).rocket + rocket_service = RocketService.from_rocket_model(flight.rocket) + rocket_service.require_motor() + rocketpy_rocket = rocket_service.rocket rocketpy_flight = RocketPyFlight( rocket=rocketpy_rocket, environment=rocketpy_env, @@ -518,9 +520,7 @@ def get_flight_kml(self) -> bytes: Returns: bytes (UTF-8 encoded KML) """ - with tempfile.NamedTemporaryFile( - suffix=".kml", delete=False - ) as tmp: + with tempfile.NamedTemporaryFile(suffix=".kml", delete=False) as tmp: tmp_path = tmp.name try: FlightDataExporter(self.flight).export_kml(file_name=tmp_path) diff --git a/src/services/motor.py b/src/services/motor.py index a7a4cb0..66ba9cd 100644 --- a/src/services/motor.py +++ b/src/services/motor.py @@ -236,14 +236,32 @@ def from_motor_model(cls, motor: MotorModel) -> Self: **optional_params, ) case _: - # GenericMotor requires burn_time even though it's optional - # for the other motor kinds — surface the constraint at the - # API boundary instead of letting rocketpy raise a - # confusing stack trace deeper in construction. - if motor.burn_time is None: + # GenericMotor has no defaults for burn_time or the chamber + # fields. A None here does not fail at construction — it + # blows up later as `Function(None)` inside + # center_of_propellant_mass when the motor is drawn, added + # to a rocket, or flown. Surface the constraint at the API + # boundary as a 422 that names every missing field, in the + # same "requires: a, b" shape the tank_kind guard uses so + # clients can paint field-level errors. + generic_params = { + 'burn_time': motor.burn_time, + 'chamber_radius': motor.chamber_radius, + 'chamber_height': motor.chamber_height, + 'chamber_position': motor.chamber_position, + 'propellant_initial_mass': motor.propellant_initial_mass, + } + missing = [ + key + for key, value in generic_params.items() + if value is None + ] + if missing: raise HTTPException( status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail="burn_time is required for generic motors.", + detail=( + "Generic motor requires: " + ', '.join(missing) + ), ) # nozzle_position is already forwarded via motor_core when # the client supplied it; GenericMotor's own default (0) diff --git a/src/services/rocket.py b/src/services/rocket.py index 3e622cb..99903d5 100644 --- a/src/services/rocket.py +++ b/src/services/rocket.py @@ -13,6 +13,7 @@ Tail as RocketPyTail, ) from rocketpy.rocket.aero_surface.generic_surface import GenericSurface +from rocketpy.motors.empty_motor import EmptyMotor from fastapi import HTTPException, status @@ -62,10 +63,13 @@ def from_rocket_model(cls, rocket: RocketModel) -> Self: center_of_mass_without_motor=rocket.center_of_mass_without_motor, coordinate_system_orientation=rocket.coordinate_system_orientation, ) - rocketpy_rocket.add_motor( - MotorService.from_motor_model(rocket.motor).motor, - rocket.motor_position, - ) + # No motor yet: keep the EmptyMotor rocketpy constructs the Rocket + # with. Drawing works; the simulate paths refuse via require_motor. + if rocket.motor is not None: + rocketpy_rocket.add_motor( + MotorService.from_motor_model(rocket.motor).motor, + rocket.motor_position, + ) # RailButtons if rocket.rail_buttons: @@ -124,12 +128,23 @@ def get_rocket_simulation(self) -> RocketSimulation: Returns: RocketSimulation """ + self.require_motor() encoded_attributes = collect_attributes( self.rocket, [RocketSimulation, MotorSimulation] ) rocket_simulation = RocketSimulation(**encoded_attributes) return rocket_simulation + def require_motor(self) -> None: + """Refuse to simulate a rocket that still carries the EmptyMotor.""" + if isinstance(self._rocket.motor, EmptyMotor): + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=( + "Rocket has no motor yet. Save a motor before simulating." + ), + ) + def get_rocket_binary(self) -> bytes: """ Get the binary representation of the rocket. diff --git a/src/views/rocket.py b/src/views/rocket.py index 1271f5f..c5e6ca3 100644 --- a/src/views/rocket.py +++ b/src/views/rocket.py @@ -94,7 +94,7 @@ class RocketSimulation(MotorSimulation): class RocketView(RocketModel): rocket_id: Optional[str] = None - motor: MotorView + motor: Optional[MotorView] = None class RocketDrawingGeometry(ApiBaseView): diff --git a/tests/unit/test_controllers/test_controller_interface.py b/tests/unit/test_controllers/test_controller_interface.py index 9cfec3f..16620c8 100644 --- a/tests/unit/test_controllers/test_controller_interface.py +++ b/tests/unit/test_controllers/test_controller_interface.py @@ -2,6 +2,7 @@ import pytest from pymongo.errors import PyMongoError from fastapi import HTTPException, status +from pydantic import BaseModel from src.controllers.interface import ( ControllerBase, controller_exception_handler, @@ -81,8 +82,10 @@ async def method(self, model, *args, **kwargs): @pytest.mark.asyncio async def test_controller_exception_handler_unexpected_exception(stub_model): + # ValueError is now a deliberate 422 (see the test below); use a + # RuntimeError as the representative programming error. async def method(self, model, *args, **kwargs): - raise ValueError('Test Error') + raise RuntimeError('Test Error') wrapped_method = controller_exception_handler(method) with patch('src.controllers.interface.logger') as mock_logger: @@ -225,3 +228,47 @@ async def test_controller_interface_delete_model(stub_controller, stub_model): ) == 'Deleted' ) + + +@pytest.mark.asyncio +async def test_controller_exception_handler_value_error_is_422(stub_model): + # RocketPy raises ValueError for physically invalid input (overfilled + # tank, bad parachute trigger, ...). That is a client error, not a + # server fault: surface the message as 422 instead of a bare 500. + async def method(self, model, *args, **kwargs): + raise ValueError( + "The tank 'Tank' is overfilled. The gas volume is greater than " + "the total volume of the tank.\n\t\tTry increasing the tank height." + ) + + wrapped_method = controller_exception_handler(method) + with patch('src.controllers.interface.logger') as mock_logger: + with pytest.raises(HTTPException) as exc: + await wrapped_method(None, stub_model) + assert exc.value.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + assert exc.value.detail.startswith("The tank 'Tank' is overfilled") + mock_logger.warning.assert_called_once() + + +@pytest.mark.asyncio +async def test_controller_exception_handler_pydantic_validation_error_stays_500( + stub_model, +): + # pydantic.ValidationError subclasses ValueError. A model failing to + # validate inside a controller means persisted data or a view is + # malformed (server fault): it must not take the 422 branch and leak the + # validation detail to the client. + class Strict(BaseModel): + radius: float + + async def method(self, model): # pylint: disable=unused-argument + Strict(radius="not a number") + + wrapped_method = controller_exception_handler(method) + with patch('src.controllers.interface.logger') as mock_logger: + with pytest.raises(HTTPException) as exc: + await wrapped_method(None, stub_model) + assert exc.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert exc.value.detail == 'Unexpected error' + mock_logger.exception.assert_called_once() + mock_logger.warning.assert_not_called() diff --git a/tests/unit/test_repositories/test_repository_interface.py b/tests/unit/test_repositories/test_repository_interface.py index f4989dc..a84670b 100644 --- a/tests/unit/test_repositories/test_repository_interface.py +++ b/tests/unit/test_repositories/test_repository_interface.py @@ -284,3 +284,51 @@ async def test_repository_find_by_query_not_found( ): assert await stub_repository.find_by_query('mock_query') == [] mock_db_interface_empty_find.find.assert_called_once_with('mock_query') + + +# --------------------------------------------------------------------------- +# Malformed ids. The jarvis client was observed calling PUT /rockets/undefined; +# bson raises InvalidId, which the generic handler turned into a 500. A +# malformed id is a client error: find returns None (→ 404 upstream), +# update/delete raise 404 directly. +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_repository_find_by_invalid_id_returns_none( + stub_repository, mock_db_interface +): + with patch( + 'src.repositories.interface.RepositoryInterface.get_collection', + return_value=mock_db_interface, + ): + assert await stub_repository.find_by_id(data_id='undefined') is None + mock_db_interface.find_one.assert_not_called() + + +@pytest.mark.asyncio +async def test_repository_update_by_invalid_id_is_404( + stub_repository, mock_db_interface +): + with patch( + 'src.repositories.interface.RepositoryInterface.get_collection', + return_value=mock_db_interface, + ): + with pytest.raises(HTTPException) as exc: + await stub_repository.update_by_id( + data_id='undefined', data={'foo': 'bar'} + ) + assert exc.value.status_code == status.HTTP_404_NOT_FOUND + mock_db_interface.update_one.assert_not_called() + + +@pytest.mark.asyncio +async def test_repository_delete_by_invalid_id_is_404( + stub_repository, mock_db_interface +): + with patch( + 'src.repositories.interface.RepositoryInterface.get_collection', + return_value=mock_db_interface, + ): + with pytest.raises(HTTPException) as exc: + await stub_repository.delete_by_id(data_id='undefined') + assert exc.value.status_code == status.HTTP_404_NOT_FOUND + mock_db_interface.delete_one.assert_not_called() diff --git a/tests/unit/test_routes/test_motors_route.py b/tests/unit/test_routes/test_motors_route.py index fe438ce..cf15cb1 100644 --- a/tests/unit/test_routes/test_motors_route.py +++ b/tests/unit/test_routes/test_motors_route.py @@ -248,16 +248,12 @@ def test_create_liquid_motor_sampled_density( assert response.status_code == 201 -def test_create_motor_invalid_geometry_kind( - stub_motor_dump, stub_tank_dump -): +def test_create_motor_invalid_geometry_kind(stub_motor_dump, stub_tank_dump): stub_tank_dump['geometry'] = { 'geometry_kind': 'pyramid', 'radius': 0.1, } - stub_motor_dump.update( - {'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'} - ) + stub_motor_dump.update({'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'}) response = client.post('/motors/', json=stub_motor_dump) assert response.status_code == 422 @@ -269,9 +265,7 @@ def test_create_motor_mass_kind_missing_fields( # populated; switching to MASS without adding liquid_mass/gas_mass # must trigger the tank_kind guard at schema validation. stub_tank_dump['tank_kind'] = 'MASS' - stub_motor_dump.update( - {'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'} - ) + stub_motor_dump.update({'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'}) response = client.post('/motors/', json=stub_motor_dump) assert response.status_code == 422 body = response.json() @@ -283,9 +277,7 @@ def test_create_motor_level_kind_missing_liquid_height( stub_motor_dump, stub_tank_dump ): stub_tank_dump['tank_kind'] = 'LEVEL' - stub_motor_dump.update( - {'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'} - ) + stub_motor_dump.update({'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'}) response = client.post('/motors/', json=stub_motor_dump) assert response.status_code == 422 assert 'liquid_height' in json.dumps(response.json()) @@ -295,17 +287,13 @@ def test_create_motor_ullage_kind_missing_ullage( stub_motor_dump, stub_tank_dump ): stub_tank_dump['tank_kind'] = 'ULLAGE' - stub_motor_dump.update( - {'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'} - ) + stub_motor_dump.update({'tanks': [stub_tank_dump], 'motor_kind': 'LIQUID'}) response = client.post('/motors/', json=stub_motor_dump) assert response.status_code == 422 assert 'ullage' in json.dumps(response.json()) -def test_create_motor_mass_flow_kind_missing_flow_rates( - stub_motor_dump -): +def test_create_motor_mass_flow_kind_missing_flow_rates(stub_motor_dump): # Build a tank payload with MASS_FLOW kind but no flow-rate fields # so the guard rejects it. tank_payload = { @@ -320,9 +308,7 @@ def test_create_motor_mass_flow_kind_missing_flow_rates( 'discretize': 0, 'tank_kind': 'MASS_FLOW', } - stub_motor_dump.update( - {'tanks': [tank_payload], 'motor_kind': 'LIQUID'} - ) + stub_motor_dump.update({'tanks': [tank_payload], 'motor_kind': 'LIQUID'}) response = client.post('/motors/', json=stub_motor_dump) assert response.status_code == 422 assert 'initial_liquid_mass' in json.dumps(response.json()) diff --git a/tests/unit/test_routes/test_rockets_route.py b/tests/unit/test_routes/test_rockets_route.py index 8139f93..99ddc71 100644 --- a/tests/unit/test_routes/test_rockets_route.py +++ b/tests/unit/test_routes/test_rockets_route.py @@ -660,3 +660,39 @@ def test_read_rocketpy_rocket_binary_server_error(mock_controller_instance): response = client.get('/rockets/123/rocketpy') assert response.status_code == 500 assert response.json() == {'detail': 'Internal Server Error'} + + +def test_create_rocket_without_nose_and_fins_is_structured_422( + stub_rocket_dump, +): + # The jarvis placeholder rocket ships with nose/fins = null. Infinity + # requires both; the rejection must come back in FastAPI's standard + # `{"detail": [{loc, msg, type}]}` shape so the client can paint the + # offending fields instead of showing "Unknown validation error". + payload = copy.deepcopy(stub_rocket_dump) + payload['nose'] = None + payload['fins'] = None + response = client.post('/rockets/', json=payload) + assert response.status_code == 422 + body = response.json() + assert isinstance(body, dict) + assert isinstance(body['detail'], list) + locs = [tuple(entry['loc']) for entry in body['detail']] + assert ('body', 'nose') in locs + assert ('body', 'fins') in locs + for entry in body['detail']: + assert set(entry) >= {'loc', 'msg', 'type'} + + +def test_create_rocket_without_motor_is_accepted( + stub_rocket_dump, mock_controller_instance +): + # A rocket saved before its motor exists goes on the wire with + # `motor: null`; the API must accept it (RocketPy uses an EmptyMotor). + payload = copy.deepcopy(stub_rocket_dump) + payload['motor'] = None + mock_controller_instance.post_rocket = AsyncMock( + return_value=RocketCreated(rocket_id='123') + ) + response = client.post('/rockets/', json=payload) + assert response.status_code == 201 diff --git a/tests/unit/test_services/test_motor_service.py b/tests/unit/test_services/test_motor_service.py index 1bb1d6c..7f5e42a 100644 --- a/tests/unit/test_services/test_motor_service.py +++ b/tests/unit/test_services/test_motor_service.py @@ -7,8 +7,10 @@ """ import pytest +from fastapi import HTTPException from rocketpy import ( CylindricalTank, + GenericMotor, Fluid, Function, LevelBasedTank, @@ -80,18 +82,14 @@ def test_cylindrical_geometry_produces_cylindrical_tank(self): assert built.height == pytest.approx(0.5) def test_spherical_geometry_produces_spherical_tank(self): - tank = _level_tank( - {'geometry_kind': 'spherical', 'radius': 0.2} - ) + tank = _level_tank({'geometry_kind': 'spherical', 'radius': 0.2}) built = _build_rocketpy_tank_geometry(tank.geometry) assert isinstance(built, SphericalTank) class TestFluidAdapter: def test_scalar_density_passes_through(self): - fluid = _build_rocketpy_fluid( - TankFluids(name='water', density=1000.0) - ) + fluid = _build_rocketpy_fluid(TankFluids(name='water', density=1000.0)) assert isinstance(fluid, Fluid) assert fluid.density == 1000.0 @@ -105,12 +103,12 @@ def test_sampled_density_becomes_callable_function(self): assert isinstance(fluid, Fluid) # The 2-input density function interpolates correctly. Rocketpy # wraps our 1D callable so pressure is accepted but ignored. - assert fluid.density_function.get_value( - 105.0, 1e5 - ) == pytest.approx(1116.0) - assert fluid.density_function.get_value( - 135.0, 1e5 - ) == pytest.approx(1056.0) + assert fluid.density_function.get_value(105.0, 1e5) == pytest.approx( + 1116.0 + ) + assert fluid.density_function.get_value(135.0, 1e5) == pytest.approx( + 1056.0 + ) class TestFromMotorModelLiquid: @@ -227,3 +225,88 @@ def test_mass_tank_keeps_sampled_density_as_function(self): ) == pytest.approx(1116.0) # Gas density stayed scalar. assert tank.gas.density == 1.2 + + +# --------------------------------------------------------------------------- +# GenericMotor guard — RocketPy's GenericMotor has no defaults for +# chamber_radius / chamber_height / chamber_position / propellant_initial_mass. +# Passing None crashes deep inside Function() when the motor is first used +# (drawing geometry, rocket assembly, flight simulation). The service must +# reject the model at the API boundary with a 422 that names the fields. +# --------------------------------------------------------------------------- + +_GENERIC_CORE = { + 'thrust_source': [[0, 1000], [1, 500]], + 'burn_time': 1.0, + 'nozzle_radius': 0.05, + 'dry_mass': 5.0, + 'center_of_dry_mass_position': 0.1, + 'motor_kind': 'GENERIC', + 'chamber_radius': 0.049, + 'chamber_height': 0.597, + 'chamber_position': 0.3, + 'propellant_initial_mass': 3.65, +} + + +class TestGenericMotorGuard: + def test_complete_generic_motor_builds(self): + service = MotorService.from_motor_model(MotorModel(**_GENERIC_CORE)) + assert isinstance(service.motor, GenericMotor) + # The crash path in production: center_of_propellant_mass must be + # computable once the chamber fields are present. + assert service.motor.center_of_propellant_mass(0.5) == pytest.approx( + 0.3 + ) + + @pytest.mark.parametrize( + 'missing', + [ + 'chamber_position', + 'chamber_radius', + 'chamber_height', + 'propellant_initial_mass', + ], + ) + def test_missing_single_generic_field_is_422(self, missing): + payload = {**_GENERIC_CORE, missing: None} + with pytest.raises(HTTPException) as exc: + MotorService.from_motor_model(MotorModel(**payload)) + assert exc.value.status_code == 422 + assert missing in exc.value.detail + assert 'requires' in exc.value.detail + + def test_missing_burn_time_and_chamber_reported_together(self): + payload = { + **_GENERIC_CORE, + 'burn_time': None, + 'chamber_position': None, + } + with pytest.raises(HTTPException) as exc: + MotorService.from_motor_model(MotorModel(**payload)) + assert exc.value.status_code == 422 + assert 'burn_time' in exc.value.detail + assert 'chamber_position' in exc.value.detail + + def test_generic_guard_does_not_apply_to_liquid(self): + # LIQUID motors legitimately carry null chamber fields. + motor_model = MotorModel( + **_LIQUID_CORE, + tanks=[ + MotorTank( + geometry={ + 'geometry_kind': 'cylindrical', + 'radius': 0.1, + 'height': 0.5, + }, + gas=TankFluids(name='N2', density=1.2), + liquid=TankFluids(name='LOX', density=1141.0), + flux_time=(0, 1.0), + position=0.5, + discretize=100, + tank_kind=TankKinds.LEVEL, + liquid_height=0.2, + ) + ], + ) + assert MotorService.from_motor_model(motor_model).motor is not None diff --git a/tests/unit/test_services/test_rocket_without_motor.py b/tests/unit/test_services/test_rocket_without_motor.py new file mode 100644 index 0000000..eab7c72 --- /dev/null +++ b/tests/unit/test_services/test_rocket_without_motor.py @@ -0,0 +1,105 @@ +"""A rocket may exist before its motor does. + +Jarvis creates the rocket entity as soon as the user saves the airframe, +often before any motor has been configured. It used to embed a placeholder +GENERIC motor with null fields, which made every drawing / simulate call +fail. RocketPy's own answer to "no motor yet" is `EmptyMotor` (a Rocket is +constructed with one by default), so Infinity accepts `motor: null` and +leaves the EmptyMotor in place. +""" + +import pytest +from fastapi import HTTPException +from rocketpy.motors.empty_motor import EmptyMotor + +from src.models.flight import FlightModel +from src.models.rocket import RocketModel +from src.services.flight import FlightService +from src.services.rocket import RocketService + +ROCKET_WITHOUT_MOTOR = { + 'motor': None, + 'radius': 0.055, + 'mass': 5.65, + 'motor_position': 0.0, + 'center_of_mass_without_motor': 0.844, + 'inertia': [1.806, 1.806, 0.0183], + 'power_off_drag': [[0, 0.35], [1, 0.35]], + 'power_on_drag': [[0, 0.35], [1, 0.35]], + 'nose': { + 'name': 'Nose', + 'length': 0.2, + 'kind': 'elliptical', + 'position': 1.84, + 'base_radius': 0.055, + 'rocket_radius': 0.055, + }, + 'fins': [ + { + 'fins_kind': 'trapezoidal', + 'name': 'Fins', + 'n': 4, + 'root_chord': 0.17, + 'tip_chord': 0.14, + 'span': 0.0768, + 'position': 0.148, + 'rocket_radius': 0.055, + } + ], + 'tail': { + 'name': 'Tail', + 'top_radius': 0.055, + 'bottom_radius': 0.03, + 'length': 0.15, + 'position': 0.0, + 'radius': 0.055, + }, +} + + +def test_model_accepts_missing_motor(): + rocket = RocketModel(**ROCKET_WITHOUT_MOTOR) + assert rocket.motor is None + omitted = {k: v for k, v in ROCKET_WITHOUT_MOTOR.items() if k != 'motor'} + assert RocketModel(**omitted).motor is None + + +def test_service_builds_rocket_with_empty_motor(): + service = RocketService.from_rocket_model( + RocketModel(**ROCKET_WITHOUT_MOTOR) + ) + assert isinstance(service.rocket.motor, EmptyMotor) + + +def test_drawing_geometry_works_without_motor(): + service = RocketService.from_rocket_model( + RocketModel(**ROCKET_WITHOUT_MOTOR) + ) + geometry = service.get_drawing_geometry() + assert len(geometry.nose_cones) == 1 + assert len(geometry.fins) == 1 + assert len(geometry.tails) == 1 + # EmptyMotor contributes no patches but the envelope stays well-formed. + assert geometry.motor is None or geometry.motor.patches == [] + + +def test_rocket_simulation_without_motor_is_422(): + service = RocketService.from_rocket_model( + RocketModel(**ROCKET_WITHOUT_MOTOR) + ) + with pytest.raises(HTTPException) as exc: + service.get_rocket_simulation() + assert exc.value.status_code == 422 + assert 'motor' in exc.value.detail.lower() + + +def test_flight_without_motor_is_422(): + flight = FlightModel( + environment={'latitude': 0.0, 'longitude': 0.0, 'elevation': 0.0}, + rocket=ROCKET_WITHOUT_MOTOR, + rail_length=5.0, + ) + with pytest.raises(HTTPException) as exc: + FlightService.from_flight_model(flight) + assert exc.value.status_code == 422 + assert 'motor' in exc.value.detail.lower()