Skip to content

Commit ccbddd7

Browse files
committed
fix(updates): drop redundant id guard in register seed path + CHANGELOG
Callers of UpdateManager::register_update (update_handlers.cpp:108) already validate that metadata["id"] is a non-empty string before invoking the manager, so the outer `if (metadata.contains("id") && metadata["id"].is_string())` check wrapping the seed block could never hit its else branch with any in-tree caller. Replace it with a direct metadata.at("id").get<std::string>() so the precondition is expressed explicitly and .at() would throw clearly if the contract were ever broken. The inner `!state_ptr` idempotency check is kept, matching the pattern in start_prepare / run_prepare. Add a CHANGELOG.rst entry under Unreleased documenting the observable contract change: GET /api/v1/updates/{id}/status returns 200 pending immediately after POST /api/v1/updates, whereas previously 404 was returned for registered-but-idle packages. 404 is now reserved for packages that are not registered.
1 parent c17eed6 commit ccbddd7

2 files changed

Lines changed: 15 additions & 12 deletions

File tree

src/ros2_medkit_gateway/CHANGELOG.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ Changelog for package ros2_medkit_gateway
55
Unreleased
66
----------
77

8+
**Fixes:**
9+
10+
* ``GET /api/v1/updates/{id}/status`` no longer returns ``404`` for a registered-but-idle package; ``POST /api/v1/updates`` now seeds a ``pending`` status, so the endpoint returns ``200 {"status": "pending"}`` immediately after registration. ``404`` is reserved for packages that are not registered. Clients that used ``404`` as a signal for "registered but nothing started yet" must adapt (`#378 <https://github.com/selfpatch/ros2_medkit/issues/378>`_)
11+
812
**Features:**
913

1014
* Plugin API version bumped to v7. Adds ``PluginContext::notify_entities_changed(EntityChangeScope)`` lifecycle hook for plugins that mutate the entity surface at runtime; default no-op keeps v6 source code compiling unchanged against v7 headers. Binary compatibility is not provided: the plugin loader uses a strict equality check on ``plugin_api_version()``, so out-of-tree plugins must be recompiled (`#376 <https://github.com/selfpatch/ros2_medkit/issues/376>`_)

src/ros2_medkit_gateway/src/updates/update_manager.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,17 @@ tl::expected<void, UpdateError> UpdateManager::register_update(const nlohmann::j
104104
// consumers (UpdatesDashboard) gate action buttons on the status field
105105
// and won't render "Prepare" when the status query fails, so a newly-
106106
// registered update would otherwise be stuck from the frontend's view.
107-
// Preserve an already-seeded state if a concurrent caller raced us
108-
// through the backend (shouldn't happen with current backends, but the
109-
// defensive check keeps the method idempotent).
110-
if (metadata.contains("id") && metadata["id"].is_string()) {
111-
const auto id = metadata["id"].get<std::string>();
112-
std::lock_guard<std::mutex> lock(mutex_);
113-
auto & state_ptr = states_[id];
114-
if (!state_ptr) {
115-
state_ptr = std::make_unique<PackageState>();
116-
state_ptr->phase = UpdatePhase::None;
117-
state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt};
118-
}
107+
// Callers (update_handlers.cpp) validate id presence + non-empty string
108+
// before invoking register_update, so metadata.at("id") is guaranteed to
109+
// hold a string at this point. The !state_ptr check matches the
110+
// idempotent-seed pattern used in start_prepare / run_prepare.
111+
const auto id = metadata.at("id").get<std::string>();
112+
std::lock_guard<std::mutex> lock(mutex_);
113+
auto & state_ptr = states_[id];
114+
if (!state_ptr) {
115+
state_ptr = std::make_unique<PackageState>();
116+
state_ptr->phase = UpdatePhase::None;
117+
state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt};
119118
}
120119
return {};
121120
}

0 commit comments

Comments
 (0)