diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 0321f22fc..977929d56 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -1772,6 +1772,45 @@ recording therefore reports its size once. One fault code can appear on several descriptors, one per occurrence it kept, told apart by ``creation_date``, which is the time that recording was made. +``size`` is the number of bytes the download route below puts on the wire for +that descriptor, so a client can size a buffer or a progress bar from the +listing. For a rosbag held in a single storage file, which is the normal case, +that is the file (``.mcap`` or ``.db3``) and it is the only file the +download serves. The bag directory also holds ``metadata.yaml``, and those bytes +are not part of the transfer. + +.. _rest-recording-size-rule: + +**One recording, one size.** The descriptor ``size`` here, the +``environment_data.snapshots[].size_bytes`` a fault reports for the same +recording, and the ``Content-Length`` of its download are the same number when +the bag's ``metadata.yaml`` names exactly one storage file, that name is a direct +child of the bag directory ending in ``.db3`` or ``.mcap``, and the file is +present - the normal case - and that number is the storage file. A recording also +has a footprint on the gateway host, which is larger because the directory holds +``metadata.yaml`` as well. That figure is what the recording spends against its +storage quota and is not reported by the API. + +Four shapes fall outside that condition, and there the three numbers part +deliberately: + +- a recording split across several storage files, past the configured maximum + bag size; +- a bag with no ``metadata.yaml``, or one that cannot be parsed; +- a bag whose named storage file is absent from the directory; +- a name that points outside the bag directory, or at a file that is neither + ``.db3`` nor ``.mcap``. The gateway and the fault manager both decline such a + name, because a name decides which file on the host is served and measured. + +In each of them the descriptor ``size`` and the nested ``size_bytes`` carry the +recording's stored total, while the download hands over whichever single storage +file the bag directory holds. ``size`` can therefore exceed ``Content-Length``, +and that gap is the signal in each of these cases: a client comparing the two can +tell the transfer it just made is a part of the recording and not the whole of +it, which no single reported number could express. When the directory holds no +storage file at all, the download has nothing to send and answers ``500``, so +there is no ``Content-Length`` to compare. + Download Bulk Data ~~~~~~~~~~~~~~~~~~ @@ -1787,6 +1826,9 @@ Download a specific bulk-data file. a pre-#620 fault-code URL is not the segment the client sent, and the format is the one persisted at capture time (``mcap`` or ``sqlite3``). For every other category it is the stored item's own name, e.g. ``report.zip``. +- ``Content-Length``: the served file's length. For how it relates to the + descriptor ``size`` of the same recording, see + :ref:`One recording, one size ` - ``Accept-Ranges``: ``bytes`` - the download is served by a range-aware provider, so a client may fetch part of the file - ``Access-Control-Expose-Headers``: ``Content-Disposition`` diff --git a/docs/tutorials/snapshots.rst b/docs/tutorials/snapshots.rst index f17d55d7e..79e0cf797 100644 --- a/docs/tutorials/snapshots.rst +++ b/docs/tutorials/snapshots.rst @@ -52,11 +52,13 @@ Quick Start ros2 launch ros2_medkit_gateway gateway.launch.py -3. **When a fault is confirmed, query its snapshots:** +3. **When a fault is confirmed, read its snapshots from the fault itself:** + + They are returned inline, under ``environment_data.snapshots``. .. code-block:: bash - curl http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots + curl http://localhost:8080/api/v1/apps/motor_controller/faults/MOTOR_OVERHEAT Configuration Options --------------------- diff --git a/postman/collections/ros2-medkit-gateway.postman_collection.json b/postman/collections/ros2-medkit-gateway.postman_collection.json index 5a138a0da..6a3b69f17 100644 --- a/postman/collections/ros2-medkit-gateway.postman_collection.json +++ b/postman/collections/ros2-medkit-gateway.postman_collection.json @@ -1355,74 +1355,6 @@ "description": "List faults with cluster details. Clusters are groups of similar faults that occurred within a time window.\n\nEach cluster includes:\n- `cluster_id`: Unique cluster identifier\n- `rule_id`, `rule_name`: The auto-cluster rule that created this cluster\n- `representative_code`: The fault shown as the cluster representative (based on rule config: first, most_recent, or highest_severity)\n- `representative_severity`: Severity of the representative fault\n- `fault_codes[]`: All fault codes in the cluster\n- `first_at`, `last_at`: Timestamps of first and last faults in cluster" }, "response": [] - }, - { - "name": "GET Fault Snapshots (System-wide)", - "request": { - "method": "GET", - "header": [], - "url": { - "raw": "{{base_url}}/faults/SENSOR_OVERTEMP/snapshots", - "host": [ - "{{base_url}}" - ], - "path": [ - "faults", - "SENSOR_OVERTEMP", - "snapshots" - ] - }, - "description": "Get topic snapshots captured when a fault transitioned to CONFIRMED status. Snapshots provide system state at the moment of fault confirmation for post-mortem debugging. Returns object with fault_code, captured_at timestamp, and topics object keyed by topic name containing message_type and parsed data." - }, - "response": [] - }, - { - "name": "GET Fault Snapshots (Filtered by Topic)", - "request": { - "method": "GET", - "header": [], - "url": { - "raw": "{{base_url}}/faults/SENSOR_OVERTEMP/snapshots?topic=/joint_states", - "host": [ - "{{base_url}}" - ], - "path": [ - "faults", - "SENSOR_OVERTEMP", - "snapshots" - ], - "query": [ - { - "key": "topic", - "value": "/joint_states" - } - ] - }, - "description": "Get snapshots filtered by specific topic name. Use this when you only need data from a particular topic." - }, - "response": [] - }, - { - "name": "GET Component Fault Snapshots", - "request": { - "method": "GET", - "header": [], - "url": { - "raw": "{{base_url}}/components/temp_sensor/faults/SENSOR_OVERTEMP/snapshots", - "host": [ - "{{base_url}}" - ], - "path": [ - "components", - "temp_sensor", - "faults", - "SENSOR_OVERTEMP", - "snapshots" - ] - }, - "description": "Get topic snapshots for a specific component's fault. Same as the system-wide endpoint but scoped to a component context. Useful when working within a component-centric workflow." - }, - "response": [] } ] }, diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp index 10e003606..5189a00db 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp @@ -487,4 +487,40 @@ class RosbagCapture { bool dynamic_discovery_{false}; }; +/// Bytes a client receives when it downloads the recording at @p bag_path. +/// +/// A recording occupies a directory and is served as a single file. Those are two +/// different quantities and the fault manager needs both. ``RosbagFileInfo::size_bytes`` +/// is the directory total, because that is what the recording costs against +/// ``max_total_storage_mb`` and what eviction frees. This is the other one: the storage +/// file the download hands over, which is what a caller sizing a buffer or a progress +/// bar needs. Reporting the total in its place overstated every download by +/// ``metadata.yaml``, and on a short recording that is around a tenth of the transfer. +/// +/// The file is the one ``metadata.yaml`` names in ``relative_file_paths``, read through +/// the same library that wrote it, so this answers with the bag's own record of its +/// contents. +/// +/// Falls back to @p stored_total_bytes, never to zero, when no single served file can +/// be named: +/// - no ``metadata.yaml``, or one that cannot be read or parsed. +/// - ``relative_file_paths`` naming other than exactly one file. Past +/// ``max_bag_size_mb`` rosbag2 splits a recording across several storage files, and +/// then no single number describes the download at all. +/// - a name that points outside the bag directory, or at something other than a +/// ``.db3`` or ``.mcap`` file. A name decides which file on the host is measured, +/// so only a storage file that is a direct child of the bag is followed. The +/// gateway applies the same rule to the same field. +/// - a named file that cannot be stat'd. +/// +/// None of those is an error worth logging. A pre-metadata bag and a split bag are +/// both normal, this runs once per reported row on every request, and the fallback is +/// a real measurement of the recording. A zero would describe the recording as +/// empty. +/// +/// @param bag_path Bag directory as stored in ``RosbagFileInfo::file_path`` +/// @param stored_total_bytes The stored directory total, used as the fallback +/// @return Size of the served storage file, or @p stored_total_bytes +size_t rosbag_served_bytes(const std::string & bag_path, size_t stored_total_bytes); + } // namespace ros2_medkit_fault_manager diff --git a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp index 146f0e3b4..962cdb691 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -1158,7 +1158,9 @@ void FaultManagerNode::handle_get_fault(const std::shared_ptrget_rosbag_file(request->fault_code); if (rosbag_info) { nlohmann::json rosbag_json; rosbag_json["available"] = true; rosbag_json["duration_sec"] = rosbag_info->duration_sec; - rosbag_json["size_bytes"] = rosbag_info->size_bytes; + rosbag_json["size_bytes"] = rosbag_served_bytes(rosbag_info->file_path, rosbag_info->size_bytes); rosbag_json["format"] = rosbag_info->format; - rosbag_json["download_url"] = "/api/v1/faults/" + request->fault_code + "/snapshots/bag"; result["rosbag"] = rosbag_json; } else { result["rosbag"] = {{"available", false}}; @@ -1598,7 +1609,7 @@ void FaultManagerNode::handle_get_rosbag(const std::shared_ptrfault_codes = attached_codes; response->format = rosbag_info->format; response->duration_sec = rosbag_info->duration_sec; - response->size_bytes = rosbag_info->size_bytes; + response->size_bytes = rosbag_served_bytes(rosbag_info->file_path, rosbag_info->size_bytes); RCLCPP_DEBUG(get_logger(), "GetRosbag returned file '%s' for %s", rosbag_info->file_path.c_str(), subject.c_str()); } @@ -1635,7 +1646,7 @@ void FaultManagerNode::handle_list_rosbags( response->file_paths.push_back(info.file_path); response->formats.push_back(info.format); response->durations_sec.push_back(info.duration_sec); - response->sizes_bytes.push_back(info.size_bytes); + response->sizes_bytes.push_back(rosbag_served_bytes(info.file_path, info.size_bytes)); response->created_at_ns.push_back(info.created_at_ns); } diff --git a/src/ros2_medkit_fault_manager/src/rosbag_capture.cpp b/src/ros2_medkit_fault_manager/src/rosbag_capture.cpp index 01d4b0c9e..b755b1f33 100644 --- a/src/ros2_medkit_fault_manager/src/rosbag_capture.cpp +++ b/src/ros2_medkit_fault_manager/src/rosbag_capture.cpp @@ -30,6 +30,8 @@ #include #include #include +#include +#include #include #include #include @@ -1263,6 +1265,37 @@ std::string fnv1a_hex(const std::string & text) { return oss.str(); } +/// The file @p relative_name names inside the bag at @p bag_path, when that is a +/// name this side follows: a direct child of the bag directory carrying a storage +/// extension, which is what rosbag2 writes. +/// +/// Path concatenation carries no such guarantee. An absolute name replaces the bag +/// directory outright and a `..` name climbs out of it, so without this test a +/// metadata.yaml chooses which file on the host gets measured and reported as the +/// recording's size. The gateway applies the same rule in its own +/// rosbag_named_storage_file (bulkdata_handlers.cpp); the two packages share no +/// code, so the rule is written out on both sides and a change to it belongs on +/// both. +/// +/// nullopt when the name is not one of those. The caller treats that as it treats +/// a named file that is absent: the stored total. +std::optional named_storage_file(const std::string & bag_path, + const std::string & relative_name) { + std::filesystem::path bag = std::filesystem::path(bag_path).lexically_normal(); + if (!bag.has_filename()) { + bag = bag.parent_path(); // tolerate a trailing slash + } + const std::filesystem::path named = (bag / relative_name).lexically_normal(); + if (named.parent_path() != bag) { + return std::nullopt; + } + const std::string extension = named.extension().string(); + if (extension != ".db3" && extension != ".mcap") { + return std::nullopt; + } + return named; +} + } // namespace std::string RosbagCapture::bag_directory_name(const std::string & fault_code, int64_t timestamp_ms) { @@ -1325,6 +1358,12 @@ std::string RosbagCapture::generate_bag_path(const std::string & fault_code) con return base_path + "/" + bag_directory_name(fault_code, timestamp); } +// The recording's footprint, and the figure stored on its rows. It is what the +// recording costs against max_total_storage_mb and what evicting it frees, so it +// counts everything in the directory including metadata.yaml and every file of a +// split. rosbag_served_bytes() below is the other measurement of the same +// recording, the one the API reports, and the two are deliberately different +// numbers - see its comment for which question each answers. size_t RosbagCapture::calculate_bag_size(const std::string & bag_path) const { size_t total_size = 0; @@ -1345,6 +1384,48 @@ size_t RosbagCapture::calculate_bag_size(const std::string & bag_path) const { return total_size; } +// The bytes a download of this recording actually transfers, which is the one +// storage file the bulk-data route hands over. calculate_bag_size() above answers +// the storage question (what the recording costs on disk) and this one answers the +// client's question (what is about to arrive). The two are separate figures, which +// is what lets the quota and the API each stay honest: a footprint reported here +// would overstate every download by metadata.yaml. +// +// The served file is the one the bag's own metadata.yaml names, and only when that +// name points at a storage file inside the bag directory. See the header for every +// fallback and why none of them logs. +size_t rosbag_served_bytes(const std::string & bag_path, size_t stored_total_bytes) { + try { + rosbag2_storage::MetadataIo metadata_io; + if (!metadata_io.metadata_file_exists(bag_path)) { + return stored_total_bytes; + } + + const rosbag2_storage::BagMetadata metadata = metadata_io.read_metadata(bag_path); + // Exactly one, or there is no single served file to measure. Zero means a bag + // that recorded nothing addressable. More than one means a split, where the + // download hands over one segment and the rest are unreachable through it - a + // defect of the download route, not something a size can paper over. + if (metadata.relative_file_paths.size() != 1) { + return stored_total_bytes; + } + + const auto storage_file = named_storage_file(bag_path, metadata.relative_file_paths.front()); + if (!storage_file) { + return stored_total_bytes; + } + std::error_code ec; + const auto served = std::filesystem::file_size(*storage_file, ec); + if (ec) { + return stored_total_bytes; + } + return static_cast(served); + } catch (const std::exception &) { + // read_metadata throws on a metadata.yaml that cannot be read or parsed. + return stored_total_bytes; + } +} + std::vector RosbagCapture::evict_bags_over_quota(FaultStorage * storage, size_t max_bytes) { size_t current_bytes = storage->get_total_rosbag_storage_bytes(); if (current_bytes <= max_bytes) { diff --git a/src/ros2_medkit_fault_manager/test/test_fault_manager.cpp b/src/ros2_medkit_fault_manager/test/test_fault_manager.cpp index 92b0abaaf..33b0d4311 100644 --- a/src/ros2_medkit_fault_manager/test/test_fault_manager.cpp +++ b/src/ros2_medkit_fault_manager/test/test_fault_manager.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -26,20 +27,25 @@ #include #include +#include +#include #include #include "rclcpp/rclcpp.hpp" #include "ros2_medkit_fault_manager/fault_audit_log.hpp" #include "ros2_medkit_fault_manager/fault_manager_node.hpp" #include "ros2_medkit_fault_manager/fault_storage.hpp" +#include "ros2_medkit_fault_manager/rosbag_capture.hpp" #include "ros2_medkit_fault_manager/sqlite_fault_storage.hpp" #include "ros2_medkit_msgs/msg/fault.hpp" #include "ros2_medkit_msgs/msg/fault_event.hpp" #include "ros2_medkit_msgs/msg/snapshot.hpp" #include "ros2_medkit_msgs/srv/clear_fault.hpp" #include "ros2_medkit_msgs/srv/get_fault.hpp" +#include "ros2_medkit_msgs/srv/get_rosbag.hpp" #include "ros2_medkit_msgs/srv/get_snapshots.hpp" #include "ros2_medkit_msgs/srv/list_faults_for_entity.hpp" +#include "ros2_medkit_msgs/srv/list_rosbags.hpp" #include "ros2_medkit_msgs/srv/report_fault.hpp" using ros2_medkit_fault_manager::clamp_debounce_counter; @@ -52,7 +58,10 @@ using ros2_medkit_msgs::msg::Fault; using ros2_medkit_msgs::msg::FaultEvent; using ros2_medkit_msgs::srv::ClearFault; using ros2_medkit_msgs::srv::GetFault; +using ros2_medkit_msgs::srv::GetRosbag; +using ros2_medkit_msgs::srv::GetSnapshots; using ros2_medkit_msgs::srv::ListFaultsForEntity; +using ros2_medkit_msgs::srv::ListRosbags; using ros2_medkit_msgs::srv::ReportFault; /// Default debounce config for tests (matches DebounceConfig defaults: threshold=-1, no healing) @@ -1551,6 +1560,199 @@ TEST_F(FreezeFrameRetentionTest, GetFaultServesRetainedFreezeFrameAfterClear) { EXPECT_DOUBLE_EQ(parsed["/ff_pressure"]["data"].get(), 91.25); } +// === Rosbag reporting through the services === + +namespace { + +/// A bag directory on disk plus the two sizes a recording has: the storage file the +/// download hands over, and the directory total the storage quota is charged. +struct ReportedBag { + explicit ReportedBag(const std::string & label) { + dir = std::filesystem::temp_directory_path() / + ("fm_reported_bag_" + std::to_string(::getpid()) + "_" + label + "_" + std::to_string(counter++)); + std::filesystem::create_directories(dir); + + { + std::ofstream out(dir / storage_file, std::ios::binary); + out << std::string(8192, 'x'); + } + rosbag2_storage::BagMetadata metadata; + metadata.storage_identifier = "sqlite3"; + metadata.relative_file_paths = {storage_file}; + metadata.duration = std::chrono::nanoseconds(0); + metadata.starting_time = std::chrono::time_point(std::chrono::nanoseconds(0)); + metadata.message_count = 0; + rosbag2_storage::MetadataIo().write_metadata(dir.string(), metadata); + + for (const auto & entry : std::filesystem::recursive_directory_iterator(dir)) { + if (entry.is_regular_file()) { + footprint += entry.file_size(); + } + } + served = std::filesystem::file_size(dir / storage_file); + } + + ~ReportedBag() { + std::error_code ec; + std::filesystem::remove_all(dir, ec); + } + + ReportedBag(const ReportedBag &) = delete; + ReportedBag & operator=(const ReportedBag &) = delete; + + /// The row the capture would have stored for @p fault_code: footprint, not served. + ros2_medkit_fault_manager::RosbagFileInfo row_for(const std::string & fault_code) const { + ros2_medkit_fault_manager::RosbagFileInfo info; + info.fault_code = fault_code; + info.file_path = dir.string(); + info.recording_id = ros2_medkit_fault_manager::rosbag_recording_id(info.file_path); + info.format = "sqlite3"; + info.duration_sec = 5.0; + info.size_bytes = footprint; + info.created_at_ns = 1738664999000000000; + return info; + } + + static constexpr const char * storage_file = "recording_0.db3"; + std::filesystem::path dir; + size_t footprint{0}; + size_t served{0}; + static int counter; +}; + +int ReportedBag::counter = 0; + +} // namespace + +// A rosbag snapshot advertises a download, so the size beside it has to be the size +// of that download. The row keeps the recording's directory total for the storage +// quota. This checks the service reports the served file instead, which is the +// wiring the helper's own unit tests in test_rosbag_capture cannot see. +TEST_F(FaultEventPublishingTest, GetFaultReportsARecordingsServedBytesNotItsFootprint) { + ReportedBag bag("get_fault"); + ASSERT_GT(bag.footprint, bag.served) << "metadata.yaml did not land, so there is nothing to tell apart"; + + ASSERT_TRUE(call_report_fault("SERVED_BYTES_FAULT", Fault::SEVERITY_ERROR, "/test_node")); + fault_manager_->get_storage_for_test().store_rosbag_file(bag.row_for("SERVED_BYTES_FAULT")); + + auto response = call_get_fault("SERVED_BYTES_FAULT"); + ASSERT_TRUE(response.has_value()); + ASSERT_TRUE(response->success); + + const ros2_medkit_msgs::msg::Snapshot * rosbag_snapshot = nullptr; + for (const auto & snapshot : response->environment_data.snapshots) { + if (snapshot.type == ros2_medkit_msgs::msg::Snapshot::TYPE_ROSBAG) { + rosbag_snapshot = &snapshot; + break; + } + } + ASSERT_NE(rosbag_snapshot, nullptr) << "the stored recording was not reported at all"; + EXPECT_EQ(rosbag_snapshot->size_bytes, bag.served) << "the snapshot must state the bytes a download transfers"; + EXPECT_NE(rosbag_snapshot->size_bytes, bag.footprint) << "the directory total is the quota's figure, not the API's"; + + // The row itself is untouched: the quota still sees the whole recording. + auto row = fault_manager_->get_storage().get_rosbag_file("SERVED_BYTES_FAULT"); + ASSERT_TRUE(row.has_value()); + EXPECT_EQ(row->size_bytes, bag.footprint) << "reporting must not have rewritten what the quota counts"; +} + +// The other three services that quote a recording's size. GetFault above covers the +// snapshot entry. These are the remaining answers, each driven through its own +// service call, so what every assertion here reads is the service's own reply. +TEST_F(FaultEventPublishingTest, EveryRosbagServiceReportsTheServedBytes) { + ReportedBag bag("all_services"); + ASSERT_GT(bag.footprint, bag.served) << "metadata.yaml did not land, so there is nothing to tell apart"; + + ASSERT_TRUE(call_report_fault("ALL_SERVICES_FAULT", Fault::SEVERITY_ERROR, "/test_node")); + const auto row = bag.row_for("ALL_SERVICES_FAULT"); + fault_manager_->get_storage_for_test().store_rosbag_file(row); + + const std::string ns = test_node_->get_namespace(); + + // GetSnapshots: the rosbag block of the JSON payload. + { + auto client = test_node_->create_client(ns + "/fault_manager/get_snapshots"); + ASSERT_TRUE(client->wait_for_service(std::chrono::seconds(5))); + auto request = std::make_shared(); + request->fault_code = "ALL_SERVICES_FAULT"; + auto future = client->async_send_request(request); + ASSERT_TRUE(spin_until_future_ready(future)); + auto response = future.get(); + ASSERT_TRUE(response->success) << response->error_message; + + auto payload = nlohmann::json::parse(response->data); + ASSERT_TRUE(payload.contains("rosbag")); + ASSERT_TRUE(payload["rosbag"].value("available", false)); + EXPECT_EQ(payload["rosbag"]["size_bytes"].get(), bag.served); + EXPECT_NE(payload["rosbag"]["size_bytes"].get(), bag.footprint); + } + + // GetRosbag: the single-recording lookup. + { + auto client = test_node_->create_client(ns + "/fault_manager/get_rosbag"); + ASSERT_TRUE(client->wait_for_service(std::chrono::seconds(5))); + auto request = std::make_shared(); + request->recording_id = row.recording_id; + request->fault_code = "ALL_SERVICES_FAULT"; + auto future = client->async_send_request(request); + ASSERT_TRUE(spin_until_future_ready(future)); + auto response = future.get(); + ASSERT_TRUE(response->success) << response->error_message; + EXPECT_EQ(response->size_bytes, bag.served); + EXPECT_NE(response->size_bytes, bag.footprint); + } + + // ListRosbags: the per-entity listing, keyed by the fault's reporting source. + { + auto client = test_node_->create_client(ns + "/fault_manager/list_rosbags"); + ASSERT_TRUE(client->wait_for_service(std::chrono::seconds(5))); + auto request = std::make_shared(); + request->entity_fqn = "/test_node"; + auto future = client->async_send_request(request); + ASSERT_TRUE(spin_until_future_ready(future)); + auto response = future.get(); + ASSERT_TRUE(response->success) << response->error_message; + ASSERT_EQ(response->sizes_bytes.size(), 1u) << "the stored recording was not listed"; + EXPECT_EQ(response->sizes_bytes[0], bag.served); + EXPECT_NE(response->sizes_bytes[0], bag.footprint); + } +} + +// The legacy /faults/{code}/snapshots/bag route was removed, so a payload naming it +// hands the caller a 404. Nothing in this repo reads the field, and a recording is +// addressed under its entity, which the fault manager cannot resolve, so the field +// is gone. +TEST_F(FaultEventPublishingTest, GetSnapshotsDoesNotAdvertiseARouteThatWasRemoved) { + ReportedBag bag("no_download_url"); + + ASSERT_TRUE(call_report_fault("NO_URL_FAULT", Fault::SEVERITY_ERROR, "/test_node")); + fault_manager_->get_storage_for_test().store_rosbag_file(bag.row_for("NO_URL_FAULT")); + + auto client = test_node_->create_client(std::string(test_node_->get_namespace()) + + "/fault_manager/get_snapshots"); + ASSERT_TRUE(client->wait_for_service(std::chrono::seconds(5))); + auto request = std::make_shared(); + request->fault_code = "NO_URL_FAULT"; + auto future = client->async_send_request(request); + ASSERT_TRUE(spin_until_future_ready(future)); + auto response = future.get(); + ASSERT_TRUE(response->success) << response->error_message; + + auto payload = nlohmann::json::parse(response->data); + + // Positive control for the absence below: the rosbag block IS present and populated, + // so a missing key is a dropped field and not an empty or absent payload. + ASSERT_TRUE(payload.contains("rosbag")) << "control: the payload carries a rosbag block"; + ASSERT_TRUE(payload["rosbag"].value("available", false)) << "control: the recording was found"; + ASSERT_TRUE(payload["rosbag"].contains("format")) << "control: the block still carries its other fields"; + + EXPECT_FALSE(payload["rosbag"].contains("download_url")) + << "the payload advertises a route that answers 404: " << payload["rosbag"].dump(); + // Nowhere else in the payload either. + EXPECT_EQ(payload.dump().find("snapshots/bag"), std::string::npos) + << "a removed route is named somewhere in the payload: " << payload.dump(); +} + // snapshots.max_per_fault and snapshots.retain_on_clear are independent settings. class UnlimitedSnapshotRetentionTest : public FaultEventPublishingTest { protected: diff --git a/src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp b/src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp index 92e81667c..3050a6be6 100644 --- a/src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp +++ b/src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp @@ -32,6 +32,8 @@ #include #include +#include +#include #include #include "rclcpp/rclcpp.hpp" @@ -566,6 +568,258 @@ TEST(RosbagHighBandwidthTopicTest, MatchesSensorStreamsButNotLookalikes) { EXPECT_FALSE(RosbagCapture::is_high_bandwidth_topic("/cmd_vel")); } +// === Reported size vs stored size === +// A recording is stored as a directory and served as a single file. The row keeps the +// directory total, because that is what the recording costs against the storage quota +// (see ABoundaryRecordingSplitsAndReportsTheWholeBag, which pins that). What the API +// reports is the other number: the bytes a download of it transfers. + +namespace { + +/// A bag directory carrying a real ``metadata.yaml``, written by the same library +/// rosbag2 writes it with, so the parse under test is the parse that runs in +/// production. +class ServedBytesBag { + public: + explicit ServedBytesBag(const std::string & label) { + dir_ = std::filesystem::temp_directory_path() / + ("served_bytes_" + std::to_string(::getpid()) + "_" + label + "_" + std::to_string(counter_++)); + std::filesystem::create_directories(dir_); + } + + ~ServedBytesBag() { + std::error_code ec; + std::filesystem::remove_all(dir_, ec); + } + + ServedBytesBag(const ServedBytesBag &) = delete; + ServedBytesBag & operator=(const ServedBytesBag &) = delete; + + /// Write a storage file of @p bytes and return its name, relative to the bag. + std::string add_storage_file(const std::string & name, size_t bytes) { + std::ofstream out(dir_ / name, std::ios::binary); + out << std::string(bytes, 'x'); + return name; + } + + void write_metadata(const std::vector & relative_file_paths) { + rosbag2_storage::BagMetadata metadata; + metadata.storage_identifier = "sqlite3"; + metadata.relative_file_paths = relative_file_paths; + metadata.duration = std::chrono::nanoseconds(0); + metadata.starting_time = std::chrono::time_point(std::chrono::nanoseconds(0)); + metadata.message_count = 0; + rosbag2_storage::MetadataIo().write_metadata(dir_.string(), metadata); + } + + /// What the row stores: every regular file under the directory. + size_t directory_total() const { + size_t total = 0; + for (const auto & entry : std::filesystem::recursive_directory_iterator(dir_)) { + if (entry.is_regular_file()) { + total += entry.file_size(); + } + } + return total; + } + + size_t file_size_of(const std::string & name) const { + return std::filesystem::file_size(dir_ / name); + } + + const std::filesystem::path & dir() const { + return dir_; + } + std::string path() const { + return dir_.string(); + } + + private: + std::filesystem::path dir_; + static int counter_; +}; + +int ServedBytesBag::counter_ = 0; + +} // namespace + +TEST(RosbagServedBytesTest, ReportsTheStorageFileNotTheDirectoryTotal) { + ServedBytesBag bag("single"); + const std::string db3 = bag.add_storage_file("recording_0.db3", 4096); + bag.write_metadata({db3}); + + const size_t served = bag.file_size_of(db3); + const size_t stored_total = bag.directory_total(); + // Not vacuous: metadata.yaml is on disk too, so the two numbers really differ. + ASSERT_GT(stored_total, served) << "metadata.yaml did not land, so there is nothing to tell apart"; + + EXPECT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total), served) + << "the reported size must be what a download transfers"; + EXPECT_NE(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total), stored_total) + << "metadata.yaml is not served, so it must not be counted"; +} + +TEST(RosbagServedBytesTest, AnUnreadableMetadataFallsBackToTheStoredTotalNotToZero) { + // Positive control on the same harness: with the metadata intact this bag does + // answer with its storage file, so a fallback below is the damaged metadata and + // not a helper that never resolves anything. + ServedBytesBag bag("damaged"); + const std::string db3 = bag.add_storage_file("recording_0.db3", 2048); + bag.write_metadata({db3}); + const size_t stored_total = bag.directory_total(); + ASSERT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total), bag.file_size_of(db3)) + << "control: an intact bag resolves its storage file"; + + // Now break only the metadata, leaving the storage file untouched. + { + std::ofstream out(bag.dir() / "metadata.yaml", std::ios::binary | std::ios::trunc); + out << "rosbag2_bagfile_information: [this is not a mapping\n"; + } + const size_t stored_total_after = bag.directory_total(); + EXPECT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total_after), stored_total_after) + << "an unparseable metadata.yaml falls back to the stored total"; + EXPECT_NE(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total_after), 0u) + << "and never to zero, which would describe the recording as empty"; +} + +TEST(RosbagServedBytesTest, AMissingMetadataFallsBackToTheStoredTotal) { + // A bag written before metadata was kept, or one whose metadata was lost. + ServedBytesBag bag("nometa"); + bag.add_storage_file("recording_0.db3", 1024); + const size_t stored_total = bag.directory_total(); + + EXPECT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total), stored_total); +} + +TEST(RosbagServedBytesTest, ANamedFileThatIsNotOnDiskFallsBackToTheStoredTotal) { + ServedBytesBag bag("ghost"); + bag.add_storage_file("recording_0.db3", 1024); + bag.write_metadata({"recording_1.db3"}); // names a segment that was never written + const size_t stored_total = bag.directory_total(); + + EXPECT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total), stored_total); +} + +namespace { + +/// A name the size helper follows only when it lands on a storage file inside the +/// bag directory. Every case below is checked twice on one bag: first with an +/// in-bag name, which must answer with that file, and then with @p +/// unfollowable_name, which must answer with the stored total. The control is what +/// separates the rule from a metadata document rosbag2 wrote but cannot read back - +/// both would fall back, for entirely different reasons. +void expect_unfollowable_name_falls_back(ServedBytesBag & bag, const std::string & unfollowable_name, + const std::string & in_bag_name) { + bag.write_metadata({in_bag_name}); + ASSERT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), bag.directory_total()), + bag.file_size_of(in_bag_name)) + << "the in-bag control name was not followed, so this bag says nothing about " << unfollowable_name; + + bag.write_metadata({unfollowable_name}); + const size_t stored_total = bag.directory_total(); + EXPECT_EQ(ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total), stored_total) + << "a name pointing outside the bag was measured as the recording: " << unfollowable_name; +} + +} // namespace + +// A name in metadata.yaml decides which file on the host this helper measures and +// reports as the recording's size, so it is followed only when it lands on a +// storage file that is a direct child of the bag directory. Path concatenation +// enforces none of that: an absolute name replaces the bag directory and a `..` +// name climbs out of it. +TEST(RosbagServedBytesTest, ANameThatClimbsOutOfTheBagFallsBackToTheStoredTotal) { + ServedBytesBag bag("escape"); + const std::string in_bag = bag.add_storage_file("recording_0.db3", 4096); + + const std::string outside_name = "served_bytes_outside_" + std::to_string(::getpid()) + ".db3"; + const auto outside = bag.dir().parent_path() / outside_name; + { + std::ofstream out(outside, std::ios::binary); + out << std::string(8192, 'o'); + } + ASSERT_TRUE(std::filesystem::is_regular_file(outside)) << "the file the name points at was not created"; + + expect_unfollowable_name_falls_back(bag, "../" + outside_name, in_bag); + + std::error_code ec; + std::filesystem::remove(outside, ec); +} + +TEST(RosbagServedBytesTest, AnAbsoluteNameFallsBackToTheStoredTotal) { + ServedBytesBag bag("absolute"); + const std::string in_bag = bag.add_storage_file("recording_0.db3", 4096); + + ServedBytesBag elsewhere("absolute_target"); + elsewhere.add_storage_file("recording_0.db3", 8192); + const std::string absolute_name = (elsewhere.dir() / "recording_0.db3").string(); + ASSERT_TRUE(std::filesystem::is_regular_file(absolute_name)); + expect_unfollowable_name_falls_back(bag, absolute_name, in_bag); + + // A host file that is not a recording at all, which is what an absolute name + // reaches for when this is an attack and not a stale bag. + expect_unfollowable_name_falls_back(bag, "/etc/hostname", in_bag); +} + +TEST(RosbagServedBytesTest, ANameInASubdirectoryFallsBackToTheStoredTotal) { + ServedBytesBag bag("nested"); + const std::string in_bag = bag.add_storage_file("recording_0.db3", 4096); + + std::filesystem::create_directories(bag.dir() / "sub"); + { + std::ofstream out(bag.dir() / "sub" / "segment.db3", std::ios::binary); + out << std::string(8192, 's'); + } + ASSERT_TRUE(std::filesystem::is_regular_file(bag.dir() / "sub" / "segment.db3")); + expect_unfollowable_name_falls_back(bag, "sub/segment.db3", in_bag); +} + +TEST(RosbagServedBytesTest, ANameWithoutAStorageExtensionFallsBackToTheStoredTotal) { + ServedBytesBag bag("text"); + const std::string in_bag = bag.add_storage_file("recording_0.db3", 4096); + bag.add_storage_file("recording_0.txt", 8192); + ASSERT_TRUE(std::filesystem::is_regular_file(bag.dir() / "recording_0.txt")); + expect_unfollowable_name_falls_back(bag, "recording_0.txt", in_bag); +} + +TEST(RosbagServedBytesTest, ASingleNamedFileIsSizedEvenWithAStrayFileBesideIt) { + // A leftover segment or a copy sitting beside the recording must not change + // which file is measured. The metadata names one file and that is the file. + // + // This is also the fault manager's half of a cross-package agreement: the + // gateway sizes the same directory shape through its own resolver, and its + // TheMetadataNamesTheStorageFileRatherThanDirectoryOrder pins the same rule: + // the size follows the file the metadata names, whichever file the directory + // yields first. Directory order decided the gateway's answer until it read + // this field too, and the two sides reported different sizes for one recording. + ServedBytesBag bag("stray"); + const std::string named = bag.add_storage_file("recording_0.db3", 4096); + bag.add_storage_file("recording_1.db3", 65536); + bag.write_metadata({named}); + const size_t stored_total = bag.directory_total(); + + const size_t reported = ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total); + EXPECT_EQ(reported, bag.file_size_of(named)); + EXPECT_EQ(reported, 4096u) << "the rule the gateway's listing follows for this shape: the file the bag names"; + EXPECT_NE(reported, bag.file_size_of("recording_1.db3")) << "the stray file is not the recording"; + EXPECT_NE(reported, stored_total); +} + +TEST(RosbagServedBytesTest, ASplitRecordingFallsBackToTheStoredTotal) { + // Past max_bag_size_mb rosbag2 splits a recording across several storage files. + // The download hands over one of them, so no single file is "the" transfer and the + // recording's own total is the only number that describes it honestly. + ServedBytesBag bag("split"); + const std::string first = bag.add_storage_file("recording_0.db3", 4096); + const std::string second = bag.add_storage_file("recording_1.db3", 2048); + bag.write_metadata({first, second}); + const size_t stored_total = bag.directory_total(); + + const size_t reported = ros2_medkit_fault_manager::rosbag_served_bytes(bag.path(), stored_total); + EXPECT_EQ(reported, stored_total); + EXPECT_NE(reported, bag.file_size_of(first)) << "picking a segment would advertise a partial recording as whole"; +} + // Fault lifecycle tests TEST_F(RosbagCaptureTest, OnFaultPrefailedWhileDisabled) { diff --git a/src/ros2_medkit_gateway/README.md b/src/ros2_medkit_gateway/README.md index 0cff2b0d3..8227c874f 100644 --- a/src/ros2_medkit_gateway/README.md +++ b/src/ros2_medkit_gateway/README.md @@ -980,13 +980,14 @@ Faults represent errors or warnings reported by system components. The gateway p - `GET /api/v1/faults` - List all faults across the system (convenience API for dashboards) - `GET /api/v1/faults/stream` - Real-time fault event stream via Server-Sent Events (SSE) -- `GET /api/v1/faults/{fault_code}/snapshots` - Get topic snapshots captured when fault was confirmed -- `GET /api/v1/faults/{fault_code}/snapshots/bag` - Download rosbag file for fault (if rosbag capture enabled) - `GET /api/v1/components/{component_id}/faults` - List faults for a specific component - `GET /api/v1/components/{component_id}/faults/{fault_code}` - Get a specific fault -- `GET /api/v1/components/{component_id}/faults/{fault_code}/snapshots` - Get snapshots for a component's fault - `DELETE /api/v1/components/{component_id}/faults/{fault_code}` - Clear a fault +Snapshots are not a separate endpoint. A fault response carries them inline in +`environment_data.snapshots[]`, and a rosbag recording is downloaded through the +bulk-data endpoints (`GET /api/v1/{entity-path}/bulk-data/rosbags/{id}`). + #### GET /api/v1/faults List all faults across the system. This is a convenience API for dashboards and monitoring tools that need a complete system health view without iterating over individual components. @@ -1103,79 +1104,12 @@ curl http://localhost:8080/api/v1/components/nav2_controller/faults } ``` -#### GET /api/v1/faults/{fault_code}/snapshots - -Get topic snapshots captured when a fault transitioned to CONFIRMED status. Snapshots provide system state at the moment of fault confirmation for debugging purposes. +#### Snapshots -**Query Parameters:** -- `topic` - (optional) Filter by specific topic name - -**Example:** -```bash -curl http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots -curl http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots?topic=/joint_states -``` - -**Response (200 OK):** -```json -{ - "fault_code": "MOTOR_OVERHEAT", - "captured_at": 1735830000.123, - "topics": { - "/joint_states": { - "message_type": "sensor_msgs/msg/JointState", - "data": {"name": ["joint1"], "position": [1.57]} - }, - "/cmd_vel": { - "message_type": "geometry_msgs/msg/Twist", - "data": {"linear": {"x": 0.5}, "angular": {"z": 0.1}} - } - } -} -``` - -**Response (200 OK - No snapshots):** -```json -{ - "fault_code": "MOTOR_OVERHEAT", - "topics": {} -} -``` - -**Response (404 Not Found):** -```json -{ - "error": "Fault not found", - "fault_code": "NONEXISTENT_FAULT" -} -``` - -#### GET /api/v1/components/{component_id}/faults/{fault_code}/snapshots - -Get topic snapshots for a specific component's fault. Same as the system-wide endpoint but scoped to a component. - -**Query Parameters:** -- `topic` - (optional) Filter by specific topic name - -**Example:** -```bash -curl http://localhost:8080/api/v1/components/motor_controller/faults/MOTOR_OVERHEAT/snapshots -``` - -**Response (200 OK):** -```json -{ - "component_id": "motor_controller", - "fault_code": "MOTOR_OVERHEAT", - "captured_at": 1735830000.123, - "topics": { - "/motor/temperature": { - "message_type": "sensor_msgs/msg/Temperature", - "data": {"temperature": 85.5, "variance": 0.1} - } - } -} -``` +Snapshots captured when a fault transitioned to CONFIRMED are returned inline +with the fault itself, in `environment_data.snapshots[]` of +`GET /api/v1/{entity-path}/faults/{fault_code}`. There is no separate snapshot +endpoint. **Snapshot Configuration:** @@ -1214,45 +1148,27 @@ default_topics: - /diagnostics ``` -#### GET /api/v1/faults/{fault_code}/snapshots/bag - -Download the rosbag file associated with a fault. This endpoint is only available when rosbag capture is enabled in FaultManager. - -Rosbag capture provides "black box" style recording - a ring buffer continuously records configured topics, and when a fault is confirmed, the buffer is flushed to a bag file. This allows capturing system state both **before and after** fault confirmation. - -**Example:** -```bash -# Download rosbag archive -curl -O -J http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots/bag - -# Or save with custom filename -curl http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots/bag -o motor_fault.tar.gz -``` - -**Response (200 OK):** -- For directory-based bags (default rosbag2 format): compressed tar.gz archive containing the full bag directory with metadata.yaml and all storage segments -- Content-Type: `application/gzip` -- Content-Disposition: `attachment; filename="fault_MOTOR_OVERHEAT_20260124_153045.tar.gz"` +#### Rosbag Recordings -The archive can be extracted and played directly with `ros2 bag play`. +Rosbag capture provides "black box" style recording - a ring buffer continuously +records configured topics, and when a fault is confirmed the buffer is flushed to +a bag file. This captures system state both **before and after** fault +confirmation. -**Response (404 Not Found - Fault or rosbag not found):** -```json -{ - "error": "Rosbag not found", - "fault_code": "MOTOR_OVERHEAT", - "details": "No rosbag file associated with this fault" -} -``` +A recording is listed and downloaded through the bulk-data endpoints: +`GET /api/v1/{entity-path}/bulk-data/rosbags` for the descriptors and +`GET /api/v1/{entity-path}/bulk-data/rosbags/{recording_id}` for the bytes. The +download serves one storage file, verbatim, named `.` +(`.mcap` or `.sqlite3`). It is not an archive and it does not include the bag's +`metadata.yaml`. -**Response (404 Not Found - Rosbag file deleted):** -```json -{ - "error": "Rosbag file not found", - "fault_code": "MOTOR_OVERHEAT", - "details": "File was deleted or moved" -} -``` +For a recording held in a single storage file, which is the normal case, the +descriptor `size` is that file's length and therefore the length of the +download. A recording that grew past `snapshots.rosbag.max_bag_size_mb` is split +across several storage files and the download hands over only one of them. The +descriptor then reports the recording's total, so `size` exceeds the download's +`Content-Length`. See [the size rule](../../docs/api/rest.rst) in the REST API +reference for the full statement. **Rosbag Configuration:** @@ -1280,17 +1196,28 @@ ros2 run ros2_medkit_fault_manager fault_manager_node \ ``` **Playback downloaded rosbag:** + +The downloaded file is a bag in itself. Point `ros2 bag` straight at it, with no +unpacking step and no `--storage` flag - rosbag2 reads the storage id out of the +file, so the same two commands work for `.mcap` and for `.sqlite3`. + ```bash -# Extract the downloaded archive -tar -xzf fault_MOTOR_OVERHEAT_20260124_153045.tar.gz +# Inspect the downloaded file +ros2 bag info fault_MOTOR_OVERHEAT_1738664999000.mcap -# Play back the bag -ros2 bag play fault_MOTOR_OVERHEAT_1735830000/ +# Play it back +ros2 bag play fault_MOTOR_OVERHEAT_1738664999000.mcap +``` -# Inspect bag contents -ros2 bag info fault_MOTOR_OVERHEAT_1735830000/ +```bash +# The same, for a recording captured with snapshots.rosbag.format: sqlite3 +ros2 bag info fault_MOTOR_OVERHEAT_1738664999000.sqlite3 +ros2 bag play fault_MOTOR_OVERHEAT_1738664999000.sqlite3 ``` +A lone storage file needs no `metadata.yaml` beside it: both commands read the +topics, the message count and the duration out of the file itself. + **Differences from JSON Snapshots:** | Feature | JSON Snapshots | Rosbag Capture | diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/bulkdata_handlers.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/bulkdata_handlers.hpp index 5786d7222..80f6ec0f0 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/bulkdata_handlers.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/bulkdata_handlers.hpp @@ -14,6 +14,8 @@ #pragma once +#include +#include #include #include #include @@ -102,6 +104,36 @@ class BulkDataHandlers { */ static std::vector download_media_types(); + /** + * @brief Resolve rosbag file path from storage path. + * + * Rosbag2 creates a directory containing the actual db3/mcap file. + * This function resolves the directory to the actual file path. A path that is + * already a regular file is returned unchanged. + * + * The single place that decides which bytes a recording IS, which is why it is + * reachable from outside the class. `download()` streams the file this returns + * and reports its length, and `detail::rosbag_served_bytes` sizes the listing + * through it, so a change to which file a recording resolves to moves both at + * once. + * + * That does not make the two numbers equal in every case, and since the split + * fix it deliberately does not. For a recording held in one storage file the + * listing resolves through here and reports exactly what the download sends. + * For a recording split across several files the listing does not come through + * here at all: it carries the recording's total from the fault manager while + * this route still hands over one file, and the gap is what tells a client the + * transfer is partial. See the size rule in ``docs/api/rest.rst``. + * + * When the bag's own ``metadata.yaml`` names exactly one storage file and that + * file exists, that is the file. Otherwise the directory is scanned for the + * first ``.db3`` or ``.mcap`` in whatever order it yields. + * + * @param path Path to rosbag (can be file or directory) + * @return Resolved file path, or empty string if not found + */ + static std::string resolve_rosbag_file_path(const std::string & path); + private: HandlerContext & ctx_; @@ -114,17 +146,6 @@ class BulkDataHandlers { * to keep the handler's public surface unchanged. */ std::vector get_source_filters(const EntityInfo & entity) const; - - /** - * @brief Resolve rosbag file path from storage path. - * - * Rosbag2 creates a directory containing the actual db3/mcap file. - * This function resolves the directory to the actual file path. - * - * @param path Path to rosbag (can be file or directory) - * @return Resolved file path, or empty string if not found - */ - static std::string resolve_rosbag_file_path(const std::string & path); }; namespace detail { @@ -205,6 +226,41 @@ std::vector rosbag_attached_fault_codes(const nlohmann::json & rosb */ bool rosbag_resolved_by_fault_code(const nlohmann::json & rosbag_data, const std::string & requested_id); +/** + * @brief Bytes a rosbag download puts on the wire for one recording. + * + * Answers only for a recording held in a single storage file, which is the only + * shape where one number describes the transfer. That is a bag directory whose + * ``metadata.yaml`` names one file, or a @p bag_path that is itself a storage + * file, which is one by definition and carries no metadata to consult. + * ``BulkDataHandlers::resolve_rosbag_file_path`` picks that file and the + * download streams it alone, so the length a client is told to expect is that + * file's length and nothing else. Reporting the bag directory's total instead + * overstated every download by ``metadata.yaml`` - on a short recording, by + * around a tenth of the transfer - and a client sizing a buffer or a progress + * bar from the listing never reached the end. + * + * Returns nullopt for anything else, and the caller then keeps the row's own + * figure. That covers a bag this process cannot see or read at all, and it + * covers a recording split across several storage files past the configured + * maximum bag size: the download hands over one segment, so no single file is + * the transfer, and answering with whichever segment the resolver reached first + * advertised a split recording at the size of one part of it. The row's figure + * is the fault manager's answer to the same question, decided from the same + * ``metadata.yaml``, so deferring to it keeps the two API surfaces agreeing on + * one recording. + * + * Never throws and never reports a filesystem error upwards. It runs once per + * row of a listing, and an error here is one unreadable recording, not a failed + * request for the entity's other recordings. + * + * @param bag_path Bag path as stored by the fault manager. A bag directory, or + * a bare storage file, which both answer + * @return The single storage file's size, or nullopt when there is not exactly + * one, or when this process cannot see it + */ +std::optional rosbag_served_bytes(const std::string & bag_path); + /** * @brief Fold rosbag link rows into one descriptor per recording. * @@ -220,6 +276,11 @@ bool rosbag_resolved_by_fault_code(const nlohmann::json & rosbag_data, const std * Order follows first appearance, which is the order the fault manager listed * the rows in. * + * The descriptor size is measured on the file the download resolves (see + * ``rosbag_served_bytes``), not taken from the row. A row whose bag this + * process cannot see keeps the row's own figure: it is the only number left, + * and a recording listed with a zero size reads as an empty one. + * * @param rows Rosbag rows as returned by the fault manager * @param faults_by_code Faults keyed by code, for timestamp enrichment * @return One descriptor per distinct recording diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/bulkdata.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/bulkdata.hpp index 69dd40d43..9c8ca8acb 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/bulkdata.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/bulkdata.hpp @@ -55,7 +55,9 @@ inline constexpr std::string_view dto_name = "BulkDataCate // id - unique file identifier (required) // name - human-readable filename / label (required) // mimetype - MIME type of the file (required) -// size - byte count (required) +// size - byte count the download route serves for this item +// (required). For a rosbag that is the bag's single +// storage file, not the bag directory's total // creation_date - ISO 8601 timestamp string (required) // description - optional human-readable description // x-medkit - optional open vendor extension object; for rosbags: diff --git a/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp index df98c93f8..a2aa17180 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp @@ -17,10 +17,12 @@ #include "ros2_medkit_gateway/core/faults/fault_scope.hpp" #include +#include #include #include #include #include +#include #include #include #include @@ -30,6 +32,7 @@ #include #include +#include #include "ros2_medkit_gateway/core/http/entity_path_utils.hpp" #include "ros2_medkit_gateway/core/http/error_codes.hpp" @@ -62,6 +65,85 @@ tl::expected parse_path(const http::TypedRequest & re return *info; } +/// The storage files the bag at @p bag_path recorded, named by its own +/// ``metadata.yaml`` in ``relative_file_paths``. +/// +/// This is the bag's own record of what it contains, and it is the same field the +/// fault manager reads for the same decisions. The same YAML parser underneath, +/// two decoders: ``rosbag2_storage::MetadataIo`` decodes the whole document into a +/// ``BagMetadata``, this reads one sequence out of it. The two agree for every +/// document both decoders accept, which is what keeps the sides describing one +/// recording alike. The gateway already links yaml-cpp, so this costs no new +/// dependency, and it does not link rosbag2_storage, which is why the field is +/// read here directly. +/// +/// nullopt when the metadata is missing, unreadable, or not the shape rosbag2 +/// writes - all of which mean the same thing to a caller, that the bag will not +/// say and the directory has to be inspected instead. +std::optional> rosbag_relative_file_paths(const std::string & bag_path) { + const std::filesystem::path metadata_path = std::filesystem::path(bag_path) / "metadata.yaml"; + std::error_code ec; + if (!std::filesystem::is_regular_file(metadata_path, ec)) { + return std::nullopt; + } + try { + const YAML::Node root = YAML::LoadFile(metadata_path.string()); + if (!root.IsMap()) { + return std::nullopt; + } + const YAML::Node info = root["rosbag2_bagfile_information"]; + if (!info || !info.IsMap()) { + return std::nullopt; + } + const YAML::Node paths = info["relative_file_paths"]; + if (!paths || !paths.IsSequence()) { + return std::nullopt; + } + std::vector names; + names.reserve(paths.size()); + for (const auto & entry : paths) { + if (!entry.IsScalar()) { + return std::nullopt; + } + names.push_back(entry.as()); + } + return names; + } catch (const std::exception &) { + return std::nullopt; + } +} + +/// The file @p relative_name names inside the bag at @p bag_path, when that is +/// a name this side will follow: a direct child of the bag directory carrying a +/// storage extension, which is what rosbag2 writes. +/// +/// The containment test is the point. ``std::filesystem::path`` concatenation +/// lets an absolute name replace the directory outright (``bag / "/etc/passwd"`` +/// is ``/etc/passwd``) and a ``..`` name climb out of it, so without this a +/// metadata.yaml would choose which file on the host the download hands over +/// and advertises a length for. The extension test keeps the same rule the +/// directory scan applies to every other candidate. +/// +/// nullopt when the name is not one of those. Callers treat that exactly as a +/// bag that named nothing: the resolver scans the directory, the size helper +/// declines. +std::optional rosbag_named_storage_file(const std::string & bag_path, + const std::string & relative_name) { + std::filesystem::path bag = std::filesystem::path(bag_path).lexically_normal(); + if (!bag.has_filename()) { + bag = bag.parent_path(); // tolerate a trailing slash + } + const std::filesystem::path named = (bag / relative_name).lexically_normal(); + if (named.parent_path() != bag) { + return std::nullopt; + } + const std::string ext = named.extension().string(); + if (ext != ".db3" && ext != ".mcap") { + return std::nullopt; + } + return named; +} + } // namespace BulkDataHandlers::BulkDataHandlers(HandlerContext & ctx) : ctx_(ctx) { @@ -86,24 +168,67 @@ std::vector BulkDataHandlers::download_media_types() { } std::string BulkDataHandlers::resolve_rosbag_file_path(const std::string & path) { + // Every filesystem call below takes the std::error_code overload, and that is + // load-bearing. This runs once per row of a bulk-data + // listing. With the throwing overloads one unreadable bag directory (EACCES), + // or one removed by quota eviction between the is_directory test and the walk + // (ENOENT), threw out of list(), which has no catch anywhere in its chain, and + // the request answered 500: a single bag nobody could read took every other + // recording of that entity out of the listing with it. Here a bag this process + // cannot read is an empty answer, not a failed request. + std::error_code ec; + // If it's a regular file, return as-is - if (std::filesystem::is_regular_file(path)) { + if (std::filesystem::is_regular_file(path, ec)) { return path; } // If it's a directory (rosbag2 directory structure), find the db3/mcap file inside - if (std::filesystem::is_directory(path)) { - for (const auto & entry : std::filesystem::directory_iterator(path)) { - if (entry.is_regular_file()) { - auto ext = entry.path().extension().string(); - // Look for db3 (sqlite3 format) or mcap files - if (ext == ".db3" || ext == ".mcap") { - return entry.path().string(); - } + if (!std::filesystem::is_directory(path, ec)) { + return ""; + } + + // Ask the bag first. When its metadata names exactly one storage file and that + // file is there, that is the file, and directory order does not get a vote. + // Iterating instead returns whichever .db3 or .mcap the directory happens to + // yield first, so a stray file beside the recording - a leftover segment, a + // copy - can be served and sized in place of the real one, while the fault + // manager, which sizes the single file it names (a split falls back to the + // stored total), reports the other. Both sides read the same field. + // + // Several names is a split recording and is deliberately left to the loop + // below: the download's choice of segment there is a separate question from + // this one. No metadata, unreadable metadata, a name this side will not follow + // and a named file that is not on disk all fall through as well, because then + // the bag has not answered. + if (const auto names = rosbag_relative_file_paths(path); names && names->size() == 1) { + if (const auto named = rosbag_named_storage_file(path, names->front())) { + std::error_code named_ec; + if (std::filesystem::is_regular_file(*named, named_ec) && !named_ec) { + return named->string(); } } } + std::filesystem::directory_iterator it(path, ec); + if (ec) { + return ""; + } + for (const std::filesystem::directory_iterator end; it != end; it.increment(ec)) { + if (ec) { + return ""; + } + std::error_code entry_ec; + if (!it->is_regular_file(entry_ec) || entry_ec) { + continue; + } + auto ext = it->path().extension().string(); + // Look for db3 (sqlite3 format) or mcap files + if (ext == ".db3" || ext == ".mcap") { + return it->path().string(); + } + } + return ""; // File not found } @@ -121,6 +246,70 @@ std::string rosbag_recording_id(const std::string & file_path) { return p.filename().string(); } +std::optional rosbag_served_bytes(const std::string & bag_path) { + if (bag_path.empty()) { + return std::nullopt; + } + + std::error_code path_ec; + const bool is_storage_file = std::filesystem::is_regular_file(bag_path, path_ec) && !path_ec; + + // Only a recording held in a single storage file has a size the download can + // be measured by. Past the configured maximum bag size rosbag2 splits a + // recording across several files and the download route hands over one of + // them, so no single file is "the" transfer, and the fault manager reports the + // recording's total for that shape. On nullopt the descriptor keeps the row's + // figure, which since the fault manager began sending served bytes IS that + // answer: the storage file for a whole recording, the directory total for a + // split one. + // + // Which file a single-file recording is held in comes from the bag's own + // metadata, and the size follows that file and no other. A named file that is + // absent leaves no answer here, the shape the fault manager answers with the + // stored total for. The resolver falls through to directory order there so the + // download still has something to hand over; that fallback does not make a + // stray file the recording's size. + // + // A bag_path that is itself a regular file IS the one storage file, and asking + // its metadata is meaningless because a file has no metadata.yaml beside it + // under that name. The path can be one: the resolver accepts a bare file and + // the download serves it, so the count gate sits behind the is_storage_file + // test - in front of it, every such row would be declined here and listed at + // zero. + std::optional> names; + if (!is_storage_file) { + names = rosbag_relative_file_paths(bag_path); + if (!names || names->size() != 1) { + return std::nullopt; + } + } + + // The same two steps `download()` performs, in the same order and through the + // same resolver, so the size a client is promised cannot drift from the size + // it is sent. Changing which file a recording resolves to changes both. + const std::string resolved = BulkDataHandlers::resolve_rosbag_file_path(bag_path); + if (resolved.empty()) { + return std::nullopt; + } + + // The measurement stands only when the file resolved IS the file the bag + // named, through the same containment rule the resolver applied, so the two + // cannot disagree about which names are followable. + if (!is_storage_file) { + const auto named = rosbag_named_storage_file(bag_path, names->front()); + if (!named || std::filesystem::path(resolved).lexically_normal() != *named) { + return std::nullopt; + } + } + + std::error_code ec; + const auto size = std::filesystem::file_size(resolved, ec); + if (ec) { + return std::nullopt; + } + return static_cast(size); +} + std::vector rosbag_attached_fault_codes(const nlohmann::json & rosbag_data, const std::string & requested_id) { if (rosbag_data.contains("fault_codes") && rosbag_data["fault_codes"].is_array()) { @@ -206,7 +395,16 @@ fold_rosbag_rows_into_descriptors(const std::vector & rows, // Default to sqlite3 (the historical FaultManager default) when a bag predates // the persisted format field; the per-bag metadata normally carries the real one. entry.format = row.value("format", "sqlite3"); - entry.size_bytes = row.value("size_bytes", uint64_t{0}); + // What the download route will actually send, measured here on the file it + // resolves. The row's figure is the fault manager's own answer to the same + // question: the storage file for a recording held in one file, the bag + // directory's total for one split across several, and the total again for a + // bag whose metadata it could not read. Measuring locally is what keeps the + // listing and the download from drifting apart on this host. Falling back to + // the row is what keeps a recording this process cannot see - a peer's bag, + // an unreadable directory, a split - described by the side that can. A zero in + // its place would describe the recording as empty. + entry.size_bytes = rosbag_served_bytes(row.value("file_path", "")).value_or(row.value("size_bytes", uint64_t{0})); entry.duration_sec = row.value("duration_sec", 0.0); entry.created_at_ns = created_at_ns; entry.fault_codes.push_back(fault_code); @@ -491,7 +689,19 @@ http::Result BulkDataHandlers::download(const http::TypedR // URL is not the segment the client sent. filename = rosbag_result.data.value("recording_id", bulk_data_id) + "." + format; - // Rosbag2 emits a directory layout - resolve the inner db3/mcap file. + // Rosbag2 emits a directory layout - resolve the inner db3/mcap file. Only + // that file is served, and metadata.yaml stays on the gateway host. + // + // For a recording whose metadata names its single storage file and that file + // is present, which is the normal case, the listing resolved this same path + // through detail::rosbag_served_bytes and the Content-Length below is the + // number it advertised. When the bag names no file this side will follow - + // no metadata, a split, a name that escapes the directory, a named file that + // is gone - the descriptor carries the total the fault manager stored while + // this route still hands over whatever the directory holds, so the two + // deliberately differ and the descriptor size exceeding Content-Length is how + // a client can tell the transfer is not the whole recording. See the size + // rule in docs/api/rest.rst. actual_path = resolve_rosbag_file_path(file_path); } else { // === Non-rosbag categories: served via BulkDataStore === diff --git a/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp b/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp index c98d65ea1..83236e1f9 100644 --- a/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp @@ -15,10 +15,19 @@ #include #include +#include +#include +#include +#include +#include +#include #include #include #include +#include +#include #include +#include #include #include "ros2_medkit_gateway/core/discovery/models/app.hpp" @@ -235,6 +244,8 @@ TEST_F(BulkDataHandlersTest, ARowWithNeitherIdNorPathIsDroppedRatherThanAdvertis } TEST_F(BulkDataHandlersTest, DistinctRecordingsEachReportTheirOwnSize) { + // The paths in these rows do not exist on this host, so each descriptor keeps + // the row's own figure - the fallback the sizing test below covers explicitly. const std::vector rows{rosbag_row("A", "fault_A_1", 2048), rosbag_row("B", "fault_B_1", 4096)}; const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors(rows, {}); @@ -247,6 +258,663 @@ TEST_F(BulkDataHandlersTest, NoRowsYieldsNoDescriptors) { EXPECT_TRUE(handlers::detail::fold_rosbag_rows_into_descriptors({}, {}).empty()); } +// === Descriptor size vs served bytes === +// A rosbag2 bag is a directory: one storage file plus metadata.yaml. The +// download resolves the storage file and streams that alone, so the descriptor +// has to be sized on the same file. The fault manager's stored figure is the +// directory total, which is the recording's disk footprint and larger than the +// transfer. Reporting it made every listing overstate the download. + +class RosbagBagDirectoryTest : public ::testing::Test { + protected: + void SetUp() override { + bag_dir_ = std::filesystem::temp_directory_path() / + ("bulkdata_bag_test_" + std::to_string(getpid()) + "_" + std::to_string(counter_++)); + std::filesystem::create_directories(bag_dir_); + write_file(bag_dir_ / "recording_0.db3", std::string(4096, 'x')); + write_metadata(bag_dir_, {"recording_0.db3"}); + } + + void TearDown() override { + std::error_code ec; + std::filesystem::remove_all(bag_dir_, ec); + } + + static void write_file(const std::filesystem::path & path, const std::string & content) { + std::ofstream out(path, std::ios::binary); + out << content; + } + + /// A ``metadata.yaml`` in the shape rosbag2 writes, naming @p storage_files in + /// ``relative_file_paths``. Only the fields this code reads are filled in, but + /// the nesting is the real one: the helper looks up + /// ``rosbag2_bagfile_information.relative_file_paths``, so a flat document + /// would pass a test that production data fails. + static void write_metadata(const std::filesystem::path & dir, const std::vector & storage_files) { + std::string yaml = + "rosbag2_bagfile_information:\n" + " version: 9\n" + " storage_identifier: sqlite3\n" + " message_count: 0\n" + " relative_file_paths:\n"; + for (const auto & file : storage_files) { + yaml += " - " + file + "\n"; + } + yaml += " ros_distro: jazzy\n"; + write_file(dir / "metadata.yaml", yaml); + } + + // What the fault manager stores: every regular file under the bag directory. + uint64_t directory_total() const { + uint64_t total = 0; + for (const auto & entry : std::filesystem::recursive_directory_iterator(bag_dir_)) { + if (entry.is_regular_file()) { + total += entry.file_size(); + } + } + return total; + } + + std::filesystem::path bag_dir_; + static int counter_; +}; + +int RosbagBagDirectoryTest::counter_ = 0; + +TEST_F(RosbagBagDirectoryTest, DescriptorSizeIsTheBytesTheDownloadServesNotTheBagDirectoryTotal) { + // The two operations download() performs to fill Content-Length: resolve the + // bag directory to its storage file, then take that file's size. + const std::string served_path = BulkDataHandlers::resolve_rosbag_file_path(bag_dir_.string()); + ASSERT_EQ(served_path, (bag_dir_ / "recording_0.db3").string()); + const uint64_t served_bytes = std::filesystem::file_size(served_path); + + // Not vacuous: the directory holds metadata.yaml as well, so the stored figure + // and the served figure are genuinely different numbers. + ASSERT_GT(directory_total(), served_bytes); + + // The row carries the directory total, which is what the fault manager stores. + const json row{{"fault_code", "MOTOR_OVERHEAT"}, + {"recording_id", bag_dir_.filename().string()}, + {"file_path", bag_dir_.string()}, + {"format", "sqlite3"}, + {"duration_sec", 6.0}, + {"size_bytes", directory_total()}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, served_bytes) << "the listing must promise the bytes the download sends"; + EXPECT_NE(descriptors[0].size, directory_total()) << "metadata.yaml is not served, so it must not be counted"; +} + +TEST_F(RosbagBagDirectoryTest, ServedBytesIsUnknownRatherThanZeroWhenTheBagIsNotVisible) { + // Positive control for the absence below: the same helper does answer for a + // bag it can see, so a nullopt is the missing bag and not a broken helper. + ASSERT_TRUE(handlers::detail::rosbag_served_bytes(bag_dir_.string()).has_value()); + + EXPECT_FALSE(handlers::detail::rosbag_served_bytes("").has_value()); + EXPECT_FALSE(handlers::detail::rosbag_served_bytes((bag_dir_ / "no_such_bag").string()).has_value()); + + // A directory with no metadata.yaml and no storage file in it. The metadata + // gate is what declines here, before the resolver is reached: the bag does not + // say how many storage files it holds, so this side will not guess one. The + // resolver would also find nothing here, which is not what this case turns on. + const auto empty_bag = bag_dir_ / "empty_bag"; + std::filesystem::create_directories(empty_bag); + EXPECT_FALSE(handlers::detail::rosbag_served_bytes(empty_bag.string()).has_value()); +} + +TEST_F(RosbagBagDirectoryTest, ASplitRecordingIsListedAtTheRowsFigureNotAtOneSegment) { + // Past the configured maximum bag size rosbag2 splits a recording across + // several storage files. The download route hands over whichever one the + // resolver reaches first, so no single file is the transfer, and sizing the + // descriptor by that file advertised a split recording at the size of one part + // of it. The fault manager reports the recording's total for a split, and the + // two API surfaces have to agree on one recording. + const auto split_dir = bag_dir_ / "split"; + std::filesystem::create_directories(split_dir); + write_file(split_dir / "split_0.db3", std::string(16384, 'a')); + write_file(split_dir / "split_1.db3", std::string(53248, 'b')); + write_metadata(split_dir, {"split_0.db3", "split_1.db3"}); + + uint64_t split_total = 0; + for (const auto & entry : std::filesystem::recursive_directory_iterator(split_dir)) { + if (entry.is_regular_file()) { + split_total += entry.file_size(); + } + } + const uint64_t first_segment = std::filesystem::file_size(split_dir / "split_0.db3"); + const uint64_t second_segment = std::filesystem::file_size(split_dir / "split_1.db3"); + + // The row carries what the fault manager reports for a split: the total. + const json row{{"fault_code", "SPLIT_FAULT"}, + {"recording_id", "fault_SPLIT_FAULT_1738664999000"}, + {"file_path", split_dir.string()}, + {"format", "sqlite3"}, + {"duration_sec", 6.0}, + {"size_bytes", split_total}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, split_total) << "a split recording keeps the figure the fault manager reported"; + EXPECT_NE(descriptors[0].size, first_segment) << "one segment is not the recording"; + EXPECT_NE(descriptors[0].size, second_segment) << "and neither is the other"; + + // The helper declines, which is what makes the fallback fire. + EXPECT_FALSE(handlers::detail::rosbag_served_bytes(split_dir.string()).has_value()); +} + +TEST_F(RosbagBagDirectoryTest, ABareStorageFileIsItsOwnRecordingAndIsSizedAsSuch) { + // A row's file_path can be the storage file itself. The resolver accepts that + // shape and the download serves it, so the listing has to size it too. A bare + // file has no metadata.yaml beside it under that name, so the metadata gate sits + // behind the is_storage_file test: in front of it, every such row is declined + // here and listed at zero the moment the row carries no figure of its own. + const auto bare_file = bag_dir_ / "standalone_recording.db3"; + write_file(bare_file, std::string(7168, 'z')); + const uint64_t bare_size = std::filesystem::file_size(bare_file); + + EXPECT_EQ(handlers::detail::rosbag_served_bytes(bare_file.string()), bare_size); + + const json row{{"fault_code", "BARE_FILE_FAULT"}, + {"recording_id", "standalone_recording.db3"}, + {"file_path", bare_file.string()}, + {"format", "sqlite3"}, + {"size_bytes", 1}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, bare_size) << "a bare storage file is measured, not declined"; + EXPECT_NE(descriptors[0].size, 1u) << "and the row's figure is not what was reported"; +} + +TEST_F(RosbagBagDirectoryTest, TheMetadataNamesTheStorageFileRatherThanDirectoryOrder) { + // A stray .db3 beside the recording - a leftover segment, a copy - is servable + // and sizeable in place of the real one for anything that takes whichever file + // the directory iterator yields first. The fault manager sizes the single file + // its metadata names, a split falling back to the stored total + // (rosbag_capture.cpp, rosbag_served_bytes), so the two sides answer + // differently for the same directory unless the gateway reads that field too. + // The bag's own metadata is the tie-break on both sides. + const auto strays = bag_dir_ / "with_stray"; + std::filesystem::create_directories(strays); + write_file(strays / "recording_0.db3", std::string(4096, 'a')); + write_file(strays / "recording_1.db3", std::string(65536, 'b')); + // Every file the bag will hold exists before the order below is sampled. + write_metadata(strays, {"recording_0.db3"}); + + // Which storage file the resolver's fallback loop reaches first here, walked + // the way that loop walks. Directory order belongs to the filesystem, so the + // test samples it: metadata naming a fixed file can agree with the loop, and + // where it agrees the metadata could be ignored entirely and the test would + // still pass. Naming the OTHER file is the naming the loop cannot reproduce. + std::filesystem::path first_in_directory_order; + for (const auto & entry : std::filesystem::directory_iterator(strays)) { + if (!entry.is_regular_file()) { + continue; + } + const auto ext = entry.path().extension().string(); + if (ext == ".db3" || ext == ".mcap") { + first_in_directory_order = entry.path(); + break; + } + } + ASSERT_FALSE(first_in_directory_order.empty()) << "no storage file was walked, so nothing is being told apart"; + + const std::filesystem::path named = first_in_directory_order.filename() == "recording_0.db3" + ? strays / "recording_1.db3" + : strays / "recording_0.db3"; + write_metadata(strays, {named.filename().string()}); + + const uint64_t named_size = std::filesystem::file_size(named); + const uint64_t stray_size = std::filesystem::file_size(first_in_directory_order); + ASSERT_NE(named_size, stray_size) << "the two files are the same size, so nothing is being told apart"; + + // The download resolves the named file, so the bytes on the wire are its bytes. + EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(strays.string()), named.string()); + EXPECT_EQ(handlers::detail::rosbag_served_bytes(strays.string()), named_size); + + const json row{{"fault_code", "STRAY_FAULT"}, + {"recording_id", "with_stray"}, + {"file_path", strays.string()}, + {"format", "sqlite3"}, + {"size_bytes", 999999}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + // The fault manager reports the size of the single file its metadata names for + // this same shape, so both sides describe one recording with one number. Its + // behaviour is pinned by ASingleNamedFileIsSizedEvenWithAStrayFileBesideIt in + // test_rosbag_capture.cpp. + EXPECT_EQ(descriptors[0].size, named_size) << "the listing must report the file the bag names"; + EXPECT_NE(descriptors[0].size, stray_size) << "directory order must not decide which file a recording is"; +} + +TEST_F(RosbagBagDirectoryTest, ANamedStorageFileThatIsGoneIsNotMeasuredByWhateverIsLeftBesideIt) { + // The bag names one storage file and that file is not there - an eviction, an + // interrupted copy - while something else with a storage extension still is. + // The resolver falls through to directory order so the download has a file to + // hand over, but a size is a promise about the recording and a stray file is + // not the recording. The fault manager answers this shape with the stored total + // (ANamedFileThatIsNotOnDiskFallsBackToTheStoredTotal in test_rosbag_capture.cpp), + // and declining here is what leaves the descriptor carrying that same figure. + const auto ghost = bag_dir_ / "ghost"; + std::filesystem::create_directories(ghost); + write_file(ghost / "recording_1.db3", std::string(65536, 'b')); + write_metadata(ghost, {"recording_0.db3"}); // a segment that was never written + + // Not vacuous: the resolver does answer here, with the file the bag never named. + ASSERT_EQ(BulkDataHandlers::resolve_rosbag_file_path(ghost.string()), (ghost / "recording_1.db3").string()); + EXPECT_FALSE(handlers::detail::rosbag_served_bytes(ghost.string()).has_value()) + << "a file the bag never named was measured as the recording"; + + const json row{{"fault_code", "GHOST_FAULT"}, + {"recording_id", "ghost"}, + {"file_path", ghost.string()}, + {"format", "sqlite3"}, + {"size_bytes", 4242}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, 4242u) << "the descriptor must keep the figure the row carried"; + EXPECT_NE(descriptors[0].size, std::filesystem::file_size(ghost / "recording_1.db3")) + << "the stray file's size reached the listing"; +} + +namespace { + +/// What this side answers for a bag whose metadata names something it will not +/// follow: the resolver hands back nothing (the directory holds no storage file +/// of its own in these shapes), the size helper declines, and the descriptor +/// keeps the figure the row carried. @p must_not_be_served is the path the name +/// points at, named here so a failure says which one leaked. +void expect_bag_names_nothing_followable(const std::filesystem::path & bag, + const std::filesystem::path & must_not_be_served, uint64_t row_figure) { + const std::string resolved = BulkDataHandlers::resolve_rosbag_file_path(bag.string()); + EXPECT_EQ(resolved, "") << "the resolver followed the metadata to " << must_not_be_served; + EXPECT_FALSE(handlers::detail::rosbag_served_bytes(bag.string()).has_value()) + << "a file outside the bag was measured as the recording"; + + const json row{{"fault_code", "UNFOLLOWABLE_NAME"}, + {"recording_id", bag.filename().string()}, + {"file_path", bag.string()}, + {"format", "sqlite3"}, + {"size_bytes", row_figure}}; + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, row_figure) << "the descriptor must keep the figure the row carried"; +} + +} // namespace + +// The four shapes below are one rule seen from four sides: the name in +// metadata.yaml chooses a file the download hands over and advertises a length +// for, so it may only ever choose a direct child of the bag directory that +// carries a storage extension - which is all rosbag2 ever writes. Path +// concatenation does not enforce that on its own: an absolute name replaces the +// directory outright and a `..` name climbs out of it, so a bag directory a +// robot can write would otherwise be able to point the route at any file the +// gateway can read. +TEST_F(RosbagBagDirectoryTest, AMetadataNameThatClimbsOutOfTheBagIsNotFollowed) { + const auto escape_target = bag_dir_ / "escape.db3"; + write_file(escape_target, std::string(8192, 'e')); + + const auto bag = bag_dir_ / "escaping_bag"; + std::filesystem::create_directories(bag); + write_metadata(bag, {"../escape.db3"}); + + // Not vacuous: the file the name points at exists and would be servable. + ASSERT_TRUE(std::filesystem::is_regular_file(escape_target)); + expect_bag_names_nothing_followable(bag, escape_target, 4242); +} + +TEST_F(RosbagBagDirectoryTest, AnAbsoluteMetadataNameIsNotFollowed) { + const auto outside = bag_dir_ / "outside_the_bag.db3"; + write_file(outside, std::string(12288, 'o')); + + const auto bag = bag_dir_ / "absolute_bag"; + std::filesystem::create_directories(bag); + write_metadata(bag, {outside.string()}); + ASSERT_TRUE(std::filesystem::is_regular_file(outside)); + expect_bag_names_nothing_followable(bag, outside, 4242); + + // The same rule with a host file that is not a recording at all, which is where + // an absolute name points when it is chosen deliberately. + write_metadata(bag, {"/etc/hostname"}); + expect_bag_names_nothing_followable(bag, "/etc/hostname", 4242); +} + +TEST_F(RosbagBagDirectoryTest, AMetadataNameInASubdirectoryIsNotFollowed) { + const auto bag = bag_dir_ / "nested_bag"; + const auto sub = bag / "sub"; + std::filesystem::create_directories(sub); + write_file(sub / "recording_0.db3", std::string(16384, 'n')); + write_metadata(bag, {"sub/recording_0.db3"}); + + // The directory scan does not descend either, so the two agree about what is + // in the bag. + ASSERT_TRUE(std::filesystem::is_regular_file(sub / "recording_0.db3")); + expect_bag_names_nothing_followable(bag, sub / "recording_0.db3", 4242); +} + +TEST_F(RosbagBagDirectoryTest, AnUnfollowableNameStillLeavesTheDownloadTheFileTheBagHolds) { + // Where the resolver and the size helper part company. A name neither will + // follow sends the resolver to the directory scan, which finds the recording + // sitting in the bag, so the download has a file to hand over. The size is a + // promise about the recording the bag claims, and the bag claims a file outside + // itself, so the listing declines and the descriptor keeps the row's figure. + const auto outside = bag_dir_ / "outside_the_bag.db3"; + write_file(outside, std::string(8192, 'o')); + + const auto bag = bag_dir_ / "bag_with_its_own_file"; + std::filesystem::create_directories(bag); + write_file(bag / "recording_0.db3", std::string(3072, 'r')); + write_metadata(bag, {"../outside_the_bag.db3"}); + + EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(bag.string()), (bag / "recording_0.db3").string()) + << "the download must be handed the storage file the bag directory holds"; + EXPECT_FALSE(handlers::detail::rosbag_served_bytes(bag.string()).has_value()) + << "a bag naming a file outside itself was given a size"; + + const json row{{"fault_code", "OUTSIDE_NAME_FAULT"}, + {"recording_id", "bag_with_its_own_file"}, + {"file_path", bag.string()}, + {"format", "sqlite3"}, + {"size_bytes", 4242}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, 4242u) << "the descriptor must keep the figure the row carried"; + EXPECT_NE(descriptors[0].size, std::filesystem::file_size(outside)) << "the file outside the bag reached the listing"; + EXPECT_NE(descriptors[0].size, std::filesystem::file_size(bag / "recording_0.db3")) + << "the served file was advertised for a recording the bag does not claim"; +} + +TEST_F(RosbagBagDirectoryTest, AMetadataNameWithoutAStorageExtensionIsNotFollowed) { + const auto bag = bag_dir_ / "text_bag"; + std::filesystem::create_directories(bag); + write_file(bag / "recording_0.txt", std::string(2048, 't')); + write_metadata(bag, {"recording_0.txt"}); + + // The directory scan accepts .db3 and .mcap only; the named file goes through + // that same gate. + ASSERT_TRUE(std::filesystem::is_regular_file(bag / "recording_0.txt")); + expect_bag_names_nothing_followable(bag, bag / "recording_0.txt", 4242); +} + +// A bag directory this process cannot walk must cost its own row and nothing +// else. The resolver used the throwing filesystem overloads, so one EACCES or +// ENOENT threw out of the listing handler, which has no catch in its chain, and +// the whole request answered 500 - every recording of the entity gone because of +// one unreadable directory. +// +// The trigger here is a symlink loop, whose ELOOP is refused whatever the caller +// holds, and these tests do not run under one identity: in CI they run as root, +// the workflow's jobs declaring a plain `container:` with no `user:` key, and +// locally as uid 1000. Root bypasses mode bits, so the mode-0000 shape only +// denies anything once the capabilities that do the bypassing are given up, which +// is what the second test below does; this one needs no such setup. +class UnreadableBagTest : public ::testing::Test { + protected: + void SetUp() override { + root_ = std::filesystem::temp_directory_path() / + ("bulkdata_unreadable_" + std::to_string(getpid()) + "_" + std::to_string(counter_++)); + std::filesystem::create_directories(root_); + + // Readable control bag: one storage file, real metadata. + readable_ = root_ / "readable_bag"; + std::filesystem::create_directories(readable_); + { + std::ofstream out(readable_ / "recording_0.db3", std::ios::binary); + out << std::string(2048, 'x'); + } + { + std::ofstream out(readable_ / "metadata.yaml", std::ios::binary); + out << "rosbag2_bagfile_information:\n version: 9\n relative_file_paths:\n - recording_0.db3\n"; + } + + // Unreadable bag: a symlink pointing at itself. Every filesystem query on it + // fails with ELOOP, for root as much as for anyone else. + loop_ = root_ / "loop_bag"; + std::error_code ec; + std::filesystem::create_symlink(loop_, loop_, ec); + symlink_created_ = !ec; + } + + void TearDown() override { + std::error_code ec; + std::filesystem::remove_all(root_, ec); + } + + std::filesystem::path root_; + std::filesystem::path readable_; + std::filesystem::path loop_; + bool symlink_created_{false}; + static int counter_; +}; + +int UnreadableBagTest::counter_ = 0; + +TEST_F(UnreadableBagTest, AnUnreadableBagCostsItsOwnRowAndNotTheListing) { + ASSERT_TRUE(symlink_created_) << "could not create the symlink loop, so nothing is being tested"; + // The loop really is refused by the filesystem, whatever uid this runs as. + std::error_code probe_ec; + const bool loop_is_a_directory = std::filesystem::is_directory(loop_, probe_ec); + ASSERT_TRUE(static_cast(probe_ec)) << "the symlink loop resolved, so it is not an unreadable bag"; + ASSERT_FALSE(loop_is_a_directory) << "a path that errored cannot also be a readable directory"; + + const uint64_t readable_served = std::filesystem::file_size(readable_ / "recording_0.db3"); + + const std::vector rows{ + json{{"fault_code", "READABLE"}, + {"recording_id", "fault_READABLE_1"}, + {"file_path", readable_.string()}, + {"format", "sqlite3"}, + {"size_bytes", 999999}}, + json{{"fault_code", "UNREADABLE"}, + {"recording_id", "fault_UNREADABLE_1"}, + {"file_path", loop_.string()}, + {"format", "sqlite3"}, + {"size_bytes", 4242}}, + }; + + // The listing answers, and answers with BOTH recordings. + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors(rows, {}); + ASSERT_EQ(descriptors.size(), 2u) << "an unreadable bag removed another recording from the listing"; + EXPECT_EQ(descriptors[0].id, "fault_READABLE_1"); + EXPECT_EQ(descriptors[0].size, readable_served) << "the readable bag is still measured locally"; + EXPECT_EQ(descriptors[1].id, "fault_UNREADABLE_1"); + EXPECT_EQ(descriptors[1].size, 4242u) << "the unreadable bag keeps the figure its row carried"; + + // These two are the assertions that pin the throwing overloads; the listing + // assertion above is defence in depth. Order is why: rosbag_served_bytes reads + // the bag's metadata before it resolves any file, and an unreadable bag has no + // readable metadata either, so it returns nullopt before the resolver is ever + // reached. download() has no such gate in front of + // it - it calls the resolver directly - so the resolver's own refusal to throw + // is what that route depends on, and it is asserted here directly. + EXPECT_NO_THROW({ EXPECT_FALSE(handlers::detail::rosbag_served_bytes(loop_.string()).has_value()); }); + EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(loop_.string()), ""); }); +} + +namespace { + +/// Everything a bag directory the caller cannot read has to survive: the size +/// helper, the resolver the download route calls directly, and the fold that +/// builds the listing from a row pointing at it. +/// +/// Returns the first reason a check did not hold, empty when all of them did. +/// The reason is what the caller reports, because not every way this body can +/// fail is a defect in the code under test: an ancestor of the bag the caller +/// cannot traverse denies everything here too, and that has to read as the +/// environment it is. +std::string locked_bag_declined_reason(const std::filesystem::path & locked, uint64_t stored_figure) { + std::string reason; + const auto check = [&reason](bool condition, const std::string & what) { + if (!condition && reason.empty()) { + reason = what; + } + }; + + try { + // The bag itself is reachable, so what denies below is the bag's own mode. + std::error_code reach_ec; + check(std::filesystem::is_directory(locked, reach_ec) && !reach_ec, + "an ancestor of " + locked.string() + " is not traversable by this caller, so the path to the bag " + + "denied before the bag's own mode could"); + + // The permission is really denied, so nothing below can pass on a directory + // that was readable all along. + std::error_code probe_ec; + std::filesystem::directory_iterator probe(locked, probe_ec); + check(static_cast(probe_ec), "the directory walked, so its mode denied nothing to this caller"); + + check(!handlers::detail::rosbag_served_bytes(locked.string()).has_value(), + "an unreadable bag was given a size of its own"); + check(BulkDataHandlers::resolve_rosbag_file_path(locked.string()).empty(), + "an unreadable bag resolved to a storage file"); + + const json row{{"fault_code", "LOCKED"}, + {"recording_id", "fault_LOCKED_1"}, + {"file_path", locked.string()}, + {"format", "sqlite3"}, + {"size_bytes", stored_figure}}; + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + check(descriptors.size() == 1u, "the listing lost the row of an unreadable bag"); + if (descriptors.size() == 1u) { + check(descriptors[0].size == stored_figure, "the unreadable bag did not keep the figure its row carried"); + } + } catch (const std::exception & e) { + check(false, std::string("a filesystem call threw: ") + e.what()); + } + return reason; +} + +/// Puts back what the locked-bag test takes away: the bag directory's mode +/// always, and the calling thread's effective capability word once +/// `arm_capabilities` has recorded one. A destructor, so an assertion that +/// returns early or an exception on any path still leaves the fixture a +/// directory it can remove and the thread the capabilities the process started +/// with - the rest of the suite runs on that thread. +class LockedBagRestore { + public: + explicit LockedBagRestore(std::filesystem::path locked) : locked_(std::move(locked)) { + } + + LockedBagRestore(const LockedBagRestore &) = delete; + LockedBagRestore & operator=(const LockedBagRestore &) = delete; + LockedBagRestore(LockedBagRestore &&) = delete; + LockedBagRestore & operator=(LockedBagRestore &&) = delete; + + ~LockedBagRestore() { + if (capabilities_armed_) { + __user_cap_header_struct header{}; + header.version = _LINUX_CAPABILITY_VERSION_3; + header.pid = 0; // the calling thread + __user_cap_data_struct capabilities[2]{}; + bool restored = ::syscall(SYS_capget, &header, capabilities) == 0; + if (restored) { + capabilities[0].effective = saved_effective_; + restored = ::syscall(SYS_capset, &header, capabilities) == 0; + } + if (!restored) { + ADD_FAILURE() << "this thread's effective capabilities could not be put back: " << std::strerror(errno); + } + } + std::error_code ec; + std::filesystem::permissions(locked_, std::filesystem::perms::owner_all, ec); + } + + void arm_capabilities(__u32 saved_effective) { + saved_effective_ = saved_effective; + capabilities_armed_ = true; + } + + private: + std::filesystem::path locked_; + __u32 saved_effective_{0}; + bool capabilities_armed_{false}; +}; + +} // namespace + +// The second shape: a directory whose mode denies everyone. Mode bits decide +// nothing for a caller holding CAP_DAC_OVERRIDE or CAP_DAC_READ_SEARCH, which +// root holds (in the CI container CAP_DAC_OVERRIDE; CAP_DAC_READ_SEARCH is not +// in Docker's default set, and clearing it is a no-op there), and this test runs +// as root in CI - the workflow's jobs declare a plain `container:` with no +// `user:` key - and as an ordinary uid locally. So it takes those two out of its +// own thread's effective set for the length of the body. Capabilities are +// per-thread and giving one up is always permitted, so this needs no second +// process: the body runs on the test's own thread, where sanitizers still check +// it and coverage still counts it. +TEST_F(UnreadableBagTest, AModeZeroDirectoryIsAlsoDeclinedRatherThanThrown) { + // Every component above the locked bag stays traversable, so what stops the + // body is the bag's own mode and not the path leading to it. + std::error_code ec; + std::filesystem::permissions( + root_, + std::filesystem::perms::owner_all | std::filesystem::perms::group_exec | std::filesystem::perms::others_exec, ec); + ASSERT_FALSE(static_cast(ec)) << "could not make the temp root traversable"; + + const auto locked = root_ / "locked_bag"; + std::filesystem::create_directories(locked); + { + std::ofstream out(locked / "recording_0.db3", std::ios::binary); + out << std::string(1024, 'x'); + } + // A complete bag, metadata included, so every answer below turns on the + // permission. + { + std::ofstream out(locked / "metadata.yaml", std::ios::binary); + out << "rosbag2_bagfile_information:\n version: 9\n relative_file_paths:\n - recording_0.db3\n"; + } + std::filesystem::permissions(locked, std::filesystem::perms::none, ec); + ASSERT_FALSE(static_cast(ec)) << "could not drop the directory's permissions"; + + constexpr uint64_t kStoredFigure = 4242; + std::string reason; + std::string capability_error; + LockedBagRestore restore(locked); + + if (::geteuid() == 0) { + __user_cap_header_struct header{}; + header.version = _LINUX_CAPABILITY_VERSION_3; + header.pid = 0; // the calling thread + __user_cap_data_struct capabilities[2]{}; + if (::syscall(SYS_capget, &header, capabilities) != 0) { + capability_error = std::string("capget: ") + std::strerror(errno); + } else { + // Effective only. The permitted set keeps both, which is what lets the + // guard put them back. + restore.arm_capabilities(capabilities[0].effective); + capabilities[0].effective &= ~((1U << CAP_DAC_OVERRIDE) | (1U << CAP_DAC_READ_SEARCH)); + if (::syscall(SYS_capset, &header, capabilities) != 0) { + capability_error = std::string("capset while dropping: ") + std::strerror(errno); + } else { + reason = locked_bag_declined_reason(locked, kStoredFigure); + } + } + } else { + reason = locked_bag_declined_reason(locked, kStoredFigure); + } + + ASSERT_TRUE(capability_error.empty()) << "this thread's capabilities could not be put where the test needs them: " + << capability_error; + EXPECT_TRUE(reason.empty()) << reason; +} + +TEST_F(RosbagBagDirectoryTest, AnUnreachableBagKeepsTheStoredFigureRatherThanReportingZero) { + const json row{{"fault_code", "MOTOR_OVERHEAT"}, + {"recording_id", "fault_MOTOR_OVERHEAT_1738664999000"}, + {"file_path", (bag_dir_ / "gone").string()}, + {"format", "sqlite3"}, + {"size_bytes", 35943}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(descriptors[0].size, 35943u); +} + // === Shared timestamp utility tests === // @verifies REQ_INTEROP_071 @@ -377,8 +1045,8 @@ TEST_F(BulkDataHandlersTest, PayloadTooLargeErrorCodeDefined) { // // Pin the entity-type branching that drives rosbag descriptor lookups + the // download ownership check. Crucial because synthetic / runtime-discovered -// components have empty fqn AND empty namespace_path: without aggregation -// from hosted apps the handler used to silently return zero source filters. +// components have empty fqn AND empty namespace_path, so the filters come from +// aggregation over the apps they host; without it the handler has none to give. // // Tested as a pure free function in detail:: against a directly-constructed // ThreadSafeEntityCache so no GatewayNode / DDS context is needed. @@ -701,7 +1369,7 @@ TEST_F(BulkDataSourceFiltersTest, ARecordingIdUrlIsNotTheCompatibilityPath) { TEST_F(BulkDataSourceFiltersTest, APeerWithoutRecordingIdsIsNotTreatedAsCompatibilityPath) { // An older peer answers by fault code only and sends no recording id. The // attached-codes fallback already reduces to the pre-#620 check there, so - // demanding a second one would 404 downloads that used to work. + // demanding a second one would 404 a download such a peer can serve. const nlohmann::json older_peer = {{"file_path", "/var/bags/fault_MOTOR_1"}}; EXPECT_FALSE(handlers::detail::rosbag_resolved_by_fault_code(older_peer, "MOTOR_OVERHEAT")); } diff --git a/src/ros2_medkit_integration_tests/test/features/test_bulk_data_api.test.py b/src/ros2_medkit_integration_tests/test/features/test_bulk_data_api.test.py index 0c70027f5..d2faf125e 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_bulk_data_api.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_bulk_data_api.test.py @@ -219,6 +219,51 @@ def test_bulk_data_unknown_category_returns_404(self): data = response.json() self.assertIn('error_code', data) + def test_bulk_data_descriptor_size_is_the_download_length(self): + """The descriptor's size is the number of bytes the download sends. + + A client sizes a buffer or a progress bar from the listing, so the + listing has to promise what the transfer delivers. Nothing else here + connects the two: the structure test above only checks that a size + field exists, and a wrong number passes that. + + The test recording is far below snapshots.rosbag.max_bag_size_mb, so it + is held in one storage file. That is the case where the descriptor size + and the download length are defined to be equal; a split recording is + reported at its total and is deliberately larger than its download. + + @verifies REQ_INTEROP_073 + """ + data = self.poll_endpoint_until( + '/apps/lidar_sensor/bulk-data/rosbags', + lambda d: d if d.get('items') else None, + timeout=10.0, + interval=1.0, + ) + self.assertGreater( + len(data['items']), 0, 'Expected at least one rosbag descriptor', + ) + descriptor = data['items'][0] + + response = requests.get( + f'{self.BASE_URL}/apps/lidar_sensor/bulk-data/rosbags/' + f'{descriptor["id"]}', + timeout=30, + ) + self.assertEqual(response.status_code, 200) + + body = response.content + self.assertGreater(len(body), 0, 'Download served an empty body') + self.assertEqual( + descriptor['size'], len(body), + f'Listing promised {descriptor["size"]} bytes and the download ' + f'sent {len(body)}', + ) + self.assertEqual( + int(response.headers['Content-Length']), len(body), + 'Content-Length disagrees with the body it described', + ) + def test_bulk_data_download_not_found(self): """Bulk-data download returns 404 for invalid UUID.