Skip to content

BUG: issues with encoding and null field updates #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

BUG: issues with encoding and null field updates #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
82ea8bb
MNT: updating objects with null initialization on edit
aasitvora99 Oct 11, 2025
44a2ebc
BUG: fix handling of corrupted Flight object in InfinityEncoder
aasitvora99 Oct 11, 2025
8cbd939
Update src/utils.py
GabrielBarberini Oct 11, 2025
9114d81
Update src/utils.py
GabrielBarberini Oct 11, 2025
cbefff8
Update src/utils.py
GabrielBarberini Oct 11, 2025
b721e28
Update src/utils.py
GabrielBarberini Oct 11, 2025
2f36252
Revert "BUG: fix handling of corrupted Flight object in InfinityEncoder"
aasitvora99 Oct 11, 2025
597aad0
BUG: fix method call for post-processing in InfinityEncoder
aasitvora99 Oct 11, 2025
ec9a552
Merge branch 'bug/simulate-forward-fixes' of github.com:RocketPy-Team…
aasitvora99 Oct 11, 2025
0286681
REV: undoing changes
aasitvora99 Oct 11, 2025
476f88b
chore: type correction
aasitvora99 Oct 11, 2025
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
2 changes: 1 addition & 1 deletion src/repositories/flight.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async def read_flight_by_id(self, flight_id: str) -> Optional[FlightModel]:
@repository_exception_handler
async def update_flight_by_id(self, flight_id: str, flight: FlightModel):
await self.update_by_id(
flight.model_dump(exclude_none=True), data_id=flight_id
flight.model_dump(exclude_none=False), data_id=flight_id
)
Comment on lines 29 to 32
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that including None-valued fields doesn't cause unintended data overwrites.

Changing exclude_none=False means None-valued fields are now included in update payloads. If the intent is partial updates (only updating provided fields), this could overwrite existing non-null database values with None.

For example, if a client sends FlightModel(environment=some_env, rocket=None), the rocket field would be set to None in the database even if a rocket was previously associated.

Confirm that one of the following is true:

  1. The update operation is meant to be a full replacement (all fields should be updated, including None values)
  2. The underlying update_by_id implementation filters out None values before persisting
  3. The API contract requires clients to always send complete objects

Run the following script to examine the update_by_id implementation:

🏁 Script executed:

#!/bin/bash
# Description: Check how update_by_id handles None values in the payload

# Find the interface implementation
ast-grep --pattern $'class RepositoryInterface:
  $$$
  async def update_by_id($$$):
    $$$'

# Also check for any None-value filtering logic
rg -nP --type=py -A10 'def update_by_id' | rg -A10 'None|null'

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for definitions of update_by_id
rg -n "async def update_by_id" -C5 --type=py
rg -n "def update_by_id" -C5 --type=py

# Search all usages to find its implementation context
rg -n "update_by_id" -C5 --type=py

Length of output: 7605

Filter out None-valued fields in the update payload
update_by_id applies {"$set": data} directly, so any keys with None from model_dump(exclude_none=False) will overwrite existing fields with null. Use model_dump(exclude_none=True) or manually remove None entries for true partial updates.

🤖 Prompt for AI Agents 
In src/repositories/flight.py around lines 29 to 32, the update payload uses
flight.model_dump(exclude_none=False) which includes None values and will
overwrite existing fields with null; change to
flight.model_dump(exclude_none=True) or filter the dict to remove keys with None
before calling update_by_id so only present fields are $set.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@aasitvora99 why is this necessary? I would prefer we drop the null fields unless their null state is actually used by the lib

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


@repository_exception_handler
Expand Down
2 changes: 1 addition & 1 deletion src/repositories/motor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async def read_motor_by_id(self, motor_id: str) -> Optional[MotorModel]:
@repository_exception_handler
async def update_motor_by_id(self, motor_id: str, motor: MotorModel):
await self.update_by_id(
motor.model_dump(exclude_none=True), data_id=motor_id
motor.model_dump(exclude_none=False), data_id=motor_id
)

@repository_exception_handler
Expand Down
2 changes: 1 addition & 1 deletion src/repositories/rocket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async def read_rocket_by_id(self, rocket_id: str) -> Optional[RocketModel]:
@repository_exception_handler
async def update_rocket_by_id(self, rocket_id: str, rocket: RocketModel):
await self.update_by_id(
rocket.model_dump(exclude_none=True), data_id=rocket_id
rocket.model_dump(exclude_none=False), data_id=rocket_id
)

@repository_exception_handler
Expand Down
8 changes: 5 additions & 3 deletions src/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ def for_flight(cls) -> 'DiscretizeConfig':


class InfinityEncoder(RocketPyEncoder):
def default(self, o):
obj = o
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

def default(self, obj):
if (
isinstance(obj, Function)
and not callable(obj.source)
Expand All @@ -73,7 +75,7 @@ def default(self, o):
mutate_self=False,
)
if isinstance(obj, Flight):
obj._Flight__evaluate_post_process()
obj._Flight__evaluate_post_process
solution = np.array(obj.solution)
size = len(solution)
if size > 25:
Expand Down
2 changes: 1 addition & 1 deletion src/views/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class EnvironmentSimulation(ApiBaseView):
initial_east: Optional[float] = None
initial_hemisphere: Optional[str] = None
initial_ew: Optional[str] = None
max_expected_height: Optional[int] = None
max_expected_height: Optional[float] = None
date: Optional[datetime] = Field(default_factory=_default_future_datetime)
local_date: Optional[datetime] = Field(
default_factory=_default_future_datetime
Expand Down
Loading