diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 47e308823..5201645f8 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -1758,7 +1758,8 @@ List all bulk-data items in a category for the entity. "fault_codes": ["MOTOR_OVERHEAT", "MOTOR_STALL"], "duration_sec": 6.0, "format": "mcap", - "recording_id": "fault_MOTOR_OVERHEAT_1738664999000" + "recording_id": "fault_MOTOR_OVERHEAT_1738664999000", + "storage_files": 1 } } ] @@ -1772,6 +1773,13 @@ 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. +``x-medkit.storage_files`` is how many storage files the recording is held in, +read from the recording's own metadata. A ``1`` is the normal case and says the +download below hands over the whole recording. A larger number is a split +recording, and then the download hands over the first of those files. The field +is omitted when the gateway cannot read the recording's metadata, so an absent +field means "not known here" rather than "one". + ``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, @@ -1792,12 +1800,28 @@ split across several storage files, past the configured maximum bag size: the download can hand over only one of them, no single file describes the transfer, and the API reports the recording's total instead. -For that split case the three numbers stop agreeing, and deliberately so. The -descriptor ``size`` and the nested ``size_bytes`` report the recording's total -while the download's ``Content-Length`` is the one storage file it hands over, -so ``size`` exceeds ``Content-Length``. That gap is the signal: a client that -compares the two can tell the transfer it just made is a part of the recording -rather than the whole of it, which no single reported number could express. +**A split recording.** Past the configured maximum bag size a recording is held +in several storage files rather than one. Three things are then true at once, +and they are meant to be read together: + +- the download hands over the **first storage file the recording's own metadata + names that is on disk**. Normally that is the first segment, where the + recording starts. A named segment that is missing from disk is skipped in + favour of the next one that is there, and when none of them is on disk the + download fails rather than serving some other file that happens to sit in the + bag directory. Which file is served does not depend on the host's filesystem + and does not change between two requests for the same recording, +- ``x-medkit.storage_files`` says **how many storage files the metadata names**, + so a client can tell it received a part and know how many parts the recording + was recorded in, +- ``size`` stays the **whole recording**, as does the nested + ``environment_data.snapshots[].size_bytes``, while the download's + ``Content-Length`` is that one file, so ``size`` exceeds ``Content-Length``. + +That last gap is a second signal for the same fact and is kept because a client +that has already made the transfer can read it without listing anything. The +storage files of a split recording after the first are not addressable over the +API today. Download Bulk Data ~~~~~~~~~~~~~~~~~~ @@ -1814,8 +1838,10 @@ 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 +- ``Content-Length``: the served file's length. For a rosbag that is the + recording's single storage file, or, when the recording is split, the first + of its storage files. For how it relates to the descriptor ``size`` and + ``x-medkit.storage_files`` 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 diff --git a/src/ros2_medkit_gateway/CHANGELOG.rst b/src/ros2_medkit_gateway/CHANGELOG.rst index 6c4dd39ee..df1ab3e61 100644 --- a/src/ros2_medkit_gateway/CHANGELOG.rst +++ b/src/ros2_medkit_gateway/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package ros2_medkit_gateway ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Downloading a rosbag recording that is split across several storage files, past ``snapshots.rosbag.max_bag_size_mb``, now hands over the first storage file the recording's own ``metadata.yaml`` names that is on disk, normally the first segment, where the recording starts. A named segment missing from disk is skipped in favour of the next one that is there, and when none of them is on disk the download fails rather than serving another file. Previously the gateway walked the bag directory and served whichever ``.db3`` or ``.mcap`` it yielded first, so which segment a client received was decided by the host's filesystem and could differ between two requests for the same recording, and a file the recording never named could be served under its id. A bag directory is now read only when the recording will not say what it holds, meaning no ``metadata.yaml``, one that cannot be read or parsed, or a ``relative_file_paths`` naming nothing. The rosbag descriptor gains ``x-medkit.storage_files``, how many storage files the recording's metadata names, so a client can tell a whole recording from one part of a split one. It is omitted in exactly those cases where the metadata does not answer, so an absent field means "not known here" rather than "one". The descriptor ``size`` is unchanged and still reports the whole recording for a split, which is why it exceeds the download's ``Content-Length``. +* The ``size`` a rosbag descriptor reports under ``GET /{entity}/bulk-data/rosbags`` is the storage file the download serves, measured through the same resolver the download uses, instead of the bag directory's total. The two differ by ``metadata.yaml``, which is never served, so every listing used to overstate its own download, 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. A bag whose ``metadata.yaml`` names exactly one storage file now resolves to that file rather than to whichever ``.db3`` or ``.mcap`` the directory yielded first, so a stray file beside the recording can no longer be served and sized in place of the real one, and the gateway and the fault manager decide it from the same field. A recording split across several storage files keeps the whole recording's figure, which is what the fault manager reports for it. A bag directory the gateway cannot read now costs its own row rather than the whole request: the resolver used the throwing filesystem calls, so one unreadable directory, or one removed by quota eviction mid-walk, answered ``500`` and took every other recording of that entity out of the listing with it. The gateway README no longer documents ``GET /faults/{code}/snapshots`` and ``.../snapshots/bag``, which have answered ``404`` since ``0.2.0``, because snapshots are returned inline with the fault and recordings are downloaded through the bulk-data endpoints. + 0.7.0 (2026-08-27) ------------------ * Rosbag bulk-data is addressed by recording id instead of fault code, so a fault holding several recordings can expose each one. ``GET /{entity}/bulk-data/rosbags`` now emits one descriptor per recording rather than one per fault - a burst that shares a bag used to appear as several entries each reporting the full bag size - and the covered faults move into ``x-medkit.fault_codes`` (was the scalar ``x-medkit.fault_code``). Old URLs keep working: an id that is not a recording is resolved as a fault code and serves that fault's newest recording, which is what it returned before. Authorization is unchanged in effect - a download is allowed when any fault the recording covers is in the entity's source scope, which is exactly the set that could reach it previously (`#623 `_, `#620 `_) diff --git a/src/ros2_medkit_gateway/README.md b/src/ros2_medkit_gateway/README.md index 8227c874f..5062ef3ec 100644 --- a/src/ros2_medkit_gateway/README.md +++ b/src/ros2_medkit_gateway/README.md @@ -1165,10 +1165,13 @@ download serves one storage file, verbatim, named `.` 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. +across several storage files, and then the download hands over the first one the +recording's own metadata names that is on disk (a named file missing from disk is +skipped, and if none is there the download fails rather than serving another +file), `x-medkit.storage_files` says how many the metadata names, and `size` +stays the whole recording, so it 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:** 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 167a5eb3a..482e95b88 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,7 @@ #pragma once +#include #include #include #include @@ -123,11 +124,32 @@ class BulkDataHandlers { * 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``. + * transfer is partial, alongside ``x-medkit.storage_files`` in the descriptor. + * 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. + * The bag's own ``metadata.yaml`` decides, and when it has decided nothing + * else gets a vote. The answer is the first file named in + * ``relative_file_paths`` that is on disk. That order is the capture order, so + * for a split recording the first name is where the recording begins and a + * client that fetches one gets its start rather than an arbitrary slice. A + * name that is not on disk is skipped, because a half-copied bag leaves + * metadata naming a file that is gone and failing the request there would cost + * a recording whose other segments are readable. A name that is absolute or + * climbs through ``..`` is skipped as well: it would resolve outside the bag + * directory and put a file that is not part of the recording on the wire. + * + * When the bag named files and none of them is on disk the answer is the empty + * string. Falling through to the directory there served whatever ``.db3`` or + * ``.mcap`` sat beside the recording, under this recording's id and against a + * ``storage_files`` count the served file is not a member of. + * + * Only when the bag will not say what it holds - no ``metadata.yaml``, one + * this process cannot read or parse, or a ``relative_file_paths`` naming + * nothing - is the directory scanned for the first ``.db3`` or ``.mcap`` in + * whatever order it yields. That is the same set of shapes + * ``detail::rosbag_storage_file_count`` declines, which is the invariant: the + * count and the served file are both read from the metadata, or both from the + * directory, never one from each. * * @param path Path to rosbag (can be file or directory) * @return Resolved file path, or empty string if not found @@ -226,6 +248,47 @@ 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 How many storage files one recording is held in. + * + * Read from the bag's own ``metadata.yaml``, the same field + * ``BulkDataHandlers::resolve_rosbag_file_path`` picks the served file out of, + * so the count and the choice of segment cannot describe different recordings. + * A @p bag_path that is itself a storage file is one by definition and carries + * no metadata beside it under that name to consult. + * + * It reaches the client as ``x-medkit.storage_files`` on the rosbag descriptor, + * and what it is for is the split case. There the descriptor ``size`` is the + * whole recording while the download hands over one segment, so ``size`` and + * ``Content-Length`` differ. Without this field that difference has no stated + * reason, and a client holding one segment has no way to learn that the rest of + * the recording exists. A ``1`` says the transfer was the whole recording. + * + * nullopt when the bag will not say - no metadata, unreadable metadata, not the + * shape rosbag2 writes, or a ``relative_file_paths`` naming nothing - and the + * field is then omitted from the descriptor rather than defaulted. Counting the + * directory's ``.db3`` / ``.mcap`` files instead would count a stray beside the + * recording, and defaulting to one would claim a recording is whole on the + * evidence of nothing. A list naming nothing is declined rather than reported as + * zero for the same reason: the recording is not empty, the bag did not answer. + * + * The set of shapes declined here is exactly the set on which + * ``BulkDataHandlers::resolve_rosbag_file_path`` falls back to the directory, so + * the count and the served file are read from the metadata together or from the + * directory together. A count taken from one source describing a file chosen by + * the other is the state this pairing exists to make unreachable. + * + * Never throws, for the same reason as the two helpers around it: it runs once + * per row of a listing, and one unreadable recording must not cost 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 number of storage files the recording names, or nullopt when the + * bag's metadata cannot be read + */ +std::optional rosbag_storage_file_count(const std::string & bag_path); + /** * @brief Bytes a rosbag download puts on the wire for one recording. * @@ -244,8 +307,8 @@ bool rosbag_resolved_by_fault_code(const nlohmann::json & rosbag_data, const std * 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 + * the transfer, and answering with the segment it hands over 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. @@ -281,6 +344,10 @@ std::optional rosbag_served_bytes(const std::string & bag_path); * 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. * + * ``x-medkit.storage_files`` carries how many files the recording is held in + * (see ``rosbag_storage_file_count``), and is omitted for a bag whose metadata + * this process cannot read. + * * @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 9c8ca8acb..679b650dd 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 @@ -56,13 +56,18 @@ inline constexpr std::string_view dto_name = "BulkDataCate // name - human-readable filename / label (required) // mimetype - MIME type of the file (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 +// (required). For a rosbag held in one storage file that is +// the file, not the bag directory's total. For one split +// across several it is the whole recording, so it exceeds +// what the download sends, which is one storage file // creation_date - ISO 8601 timestamp string (required) // description - optional human-readable description -// x-medkit - optional open vendor extension object; for rosbags: -// {fault_code, duration_sec, format}; for user uploads: -// arbitrary metadata JSON object set by the uploader. +// x-medkit - optional open vendor extension object. For rosbags: +// {fault_codes, duration_sec, format, recording_id, +// storage_files}, the last of which is how many storage +// files the recording's metadata names and is omitted when +// that metadata cannot be read. For user uploads: an arbitrary +// metadata JSON object set by the uploader. // ============================================================================= struct BulkDataDescriptor { std::string id; 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 730103a70..05e5473f8 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp @@ -112,6 +112,30 @@ std::optional> rosbag_relative_file_paths(const std::st } } +/// Does @p name, joined onto the bag directory, stay inside it? +/// +/// The names come out of a ``metadata.yaml`` on disk and are joined onto the bag +/// path, so they are input rather than a constant. An absolute name replaces the +/// bag path outright, because that is what ``operator/`` does with an absolute +/// right-hand side, and a name climbing through ``..`` walks out of the +/// directory. Either way the resolver would return a path outside the recording +/// and the download would stream that file under the recording's id. +/// +/// rosbag2 writes plain basenames here, so nothing legitimate is refused. This +/// is about what a bag directory that is not what it claims can name. +bool rosbag_name_stays_in_bag(const std::string & name) { + const std::filesystem::path candidate(name); + if (candidate.is_absolute() || candidate.has_root_name()) { + return false; + } + for (const auto & part : candidate) { + if (part == "..") { + return false; + } + } + return true; +} + } // namespace BulkDataHandlers::BulkDataHandlers(HandlerContext & ctx) : ctx_(ctx) { @@ -156,24 +180,56 @@ std::string BulkDataHandlers::resolve_rosbag_file_path(const std::string & path) 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 returned whichever .db3 or .mcap the directory happened to - // yield first, so a stray file beside the recording - a leftover segment, a - // copy - could be served and sized in place of the real one, while the fault + // Ask the bag first. Its metadata names the storage files it holds, in the + // order they were recorded, and the first of those that is on disk is the file + // to hand over. Directory order does not get a vote. + // + // For a recording held in one file that removes a stray .db3 beside it - a + // leftover segment, a copy - from being served and sized in place of the real + // one, which is what the directory walk below could pick while the fault // manager, which sizes relative_file_paths.front(), reported the other. Same - // question, same evidence, on both sides now. + // question, same evidence, on both sides. + // + // For a recording split across several files it decides which segment the + // download is. The walk below answered with whichever .db3 or .mcap the + // directory happened to yield first, which is a segment from the middle of the + // recording as readily as its start, and which file that was could change + // between two requests for the same bag. The metadata's order is the capture + // order, so its first name is where the recording begins, and a client that + // fetches a split recording gets its start rather than an arbitrary slice. + // ``x-medkit.storage_files`` in the descriptor is what tells that client the + // rest of the recording exists (see rosbag_storage_file_count). + // + // A name that is not on disk is skipped rather than returned: a half-copied + // bag leaves metadata naming a file that is gone, and resolving to it would + // fail the request for a recording whose other segments are readable. + // + // When the bag HAS named files and none of them is on disk, the answer is + // nothing. The directory is deliberately not consulted then: it is the same + // question the bag already answered, and reaching past that answer served + // whatever .db3 or .mcap happened to sit beside the recording - a stray, a + // copy, a segment of a different bag - under this recording's id, and counted + // in ``x-medkit.storage_files`` a list the served file is not a member of. A + // client received neither the recording nor an error, and nothing in the + // response said which. Empty is honest: the listing keeps the row's own figure + // and the download answers its own error. // - // 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, or 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) { - const std::filesystem::path named = std::filesystem::path(path) / names->front(); - std::error_code named_ec; - if (std::filesystem::is_regular_file(named, named_ec) && !named_ec) { - return named.string(); + // The walk below is for a bag that will not say what it holds at all: no + // metadata, metadata this process cannot read or parse, or a list naming + // nothing (see rosbag_storage_file_count, which declines the same shapes). + // Metadata decides both the count and the served file, or neither does. + if (const auto names = rosbag_relative_file_paths(path); names && !names->empty()) { + for (const auto & name : *names) { + if (!rosbag_name_stays_in_bag(name)) { + continue; + } + const std::filesystem::path named = std::filesystem::path(path) / name; + std::error_code named_ec; + if (std::filesystem::is_regular_file(named, named_ec) && !named_ec) { + return named.string(); + } } + return ""; } std::filesystem::directory_iterator it(path, ec); @@ -212,6 +268,36 @@ std::string rosbag_recording_id(const std::string & file_path) { return p.filename().string(); } +std::optional rosbag_storage_file_count(const std::string & bag_path) { + if (bag_path.empty()) { + return std::nullopt; + } + + // A path that is already a storage file is one storage file, and has no + // metadata.yaml beside it under that name to ask. The resolver accepts such a + // path and the download serves it, so it is a shape the descriptor describes. + std::error_code ec; + if (std::filesystem::is_regular_file(bag_path, ec) && !ec) { + return std::size_t{1}; + } + + // Anything else is answered by the bag's own record or not at all. A directory + // walk would count strays beside the recording, and a bag this process cannot + // read would count zero, which reads as an empty recording rather than as an + // unread one. + // + // A list naming nothing is that same zero by another route, so it is declined + // here rather than reported: the recording is not empty, the bag simply did + // not say what it holds. The resolver treats that shape identically and falls + // back to the directory, which keeps the invariant that the count and the + // served file come from the metadata together or from the directory together. + const auto names = rosbag_relative_file_paths(bag_path); + if (!names || names->empty()) { + return std::nullopt; + } + return names->size(); +} + std::optional rosbag_served_bytes(const std::string & bag_path) { if (bag_path.empty()) { return std::nullopt; @@ -295,6 +381,7 @@ fold_rosbag_rows_into_descriptors(const std::vector & rows, std::string recording_id; std::string format; uint64_t size_bytes{0}; + std::optional storage_files; double duration_sec{0.0}; int64_t created_at_ns{0}; std::vector fault_codes; @@ -355,6 +442,11 @@ fold_rosbag_rows_into_descriptors(const std::vector & rows, // a zero instead would describe the recording as empty rather than as // unmeasured here. entry.size_bytes = rosbag_served_bytes(row.value("file_path", "")).value_or(row.value("size_bytes", uint64_t{0})); + // How many storage files the recording holds, so a client can tell a whole + // recording from one segment of a split. Without it the descriptor size and + // the download's Content-Length differ with no stated reason, and a client + // that fetched a split has no way to learn that more of it exists. + entry.storage_files = rosbag_storage_file_count(row.value("file_path", "")); entry.duration_sec = row.value("duration_sec", 0.0); entry.created_at_ns = created_at_ns; entry.fault_codes.push_back(fault_code); @@ -380,6 +472,12 @@ fold_rosbag_rows_into_descriptors(const std::vector & rows, // Redundant with the descriptor id, kept because // clients already group on it. {"recording_id", entry.recording_id}}; + // Omitted rather than defaulted when the bag would not say: a one there + // would claim the recording is whole, which is the one thing this field + // exists to establish. + if (entry.storage_files) { + (*descriptor.x_medkit)["storage_files"] = *entry.storage_files; + } descriptors.push_back(std::move(descriptor)); } return descriptors; diff --git a/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp b/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp index 9395c4e2f..a72e799d7 100644 --- a/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_bulkdata_handlers.cpp @@ -285,20 +285,54 @@ class RosbagBagDirectoryTest : public ::testing::Test { /// 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. + /// + /// An empty @p storage_files is written as the flow-style ``[]``, which is an + /// empty sequence. A block sequence with no items under it is a YAML *null* + /// instead, and a null is refused one step earlier than an empty list is, by + /// the shape check rather than by the count, so a test built on one passes + /// without ever reaching the rule it names. 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"; + " message_count: 0\n"; + if (storage_files.empty()) { + yaml += " relative_file_paths: []\n"; + } else { + yaml += " relative_file_paths:\n"; + for (const auto & file : storage_files) { + yaml += " - " + file + "\n"; + } } yaml += " ros_distro: jazzy\n"; write_file(dir / "metadata.yaml", yaml); } + /// The storage file the directory-order fallback inside + /// ``resolve_rosbag_file_path`` reaches first: the first regular ``.db3`` or + /// ``.mcap`` the directory yields. Empty when the directory holds none. + /// + /// The split tests use this to choose a metadata order that cannot coincide + /// with the directory's. Which file a directory yields first is the + /// filesystem's own business - it is neither creation order nor lexical order + /// on the overlay these tests run on - so a test that writes down an expected + /// answer and hopes it differs from directory order proves nothing on the run + /// where the two agree. Asking at run time makes "the bag's own order decides" + /// falsifiable everywhere. + static std::filesystem::path first_in_directory_order(const std::filesystem::path & dir) { + for (const auto & entry : std::filesystem::directory_iterator(dir)) { + if (!entry.is_regular_file()) { + continue; + } + const auto ext = entry.path().extension().string(); + if (ext == ".db3" || ext == ".mcap") { + return entry.path(); + } + } + return {}; + } + // What the fault manager stores: every regular file under the bag directory. uint64_t directory_total() const { uint64_t total = 0; @@ -462,6 +496,364 @@ TEST_F(RosbagBagDirectoryTest, TheMetadataNamesTheStorageFileRatherThanDirectory EXPECT_NE(descriptors[0].size, stray_size) << "directory order must not decide which file a recording is"; } +// === A split recording: which segment is downloaded, and how many there are === +// Past the configured maximum bag size rosbag2 splits a recording across several +// storage files. The download hands over one of them, and that used to be +// whichever the directory iterator yielded first - a segment from the middle of +// the recording as readily as its start, decided by nothing a client could see +// or predict. The recording's own metadata.yaml lists its segments in capture +// order, so the first name in it is where the recording starts, and that is the +// file to hand over. The count goes into the descriptor because a client holding +// one segment has no other way to learn that more of the recording exists. + +TEST_F(RosbagBagDirectoryTest, ASplitRecordingResolvesToTheFirstSegmentTheMetadataNames) { + // Three segments, and the one the metadata names first is chosen so that + // neither of the rules this one replaces can reach it: it is not the file the + // directory yields first, and it is not the lexically smallest. So a resolver + // that walks the directory, and a resolver that sorts, both have to fail here. + const auto split_dir = bag_dir_ / "split_ordered"; + std::filesystem::create_directories(split_dir); + const std::vector segments{split_dir / "recording_0.db3", split_dir / "recording_1.db3", + split_dir / "recording_2.db3"}; + write_file(segments[0], std::string(16384, 'a')); + write_file(segments[1], std::string(53248, 'b')); + write_file(segments[2], std::string(32768, 'c')); + // Written before the order is read, and rewritten in place afterwards, so + // adding metadata.yaml cannot move the entries the answer was read from. + write_metadata(split_dir, {"recording_0.db3"}); + + const auto directory_first = first_in_directory_order(split_dir); + ASSERT_FALSE(directory_first.empty()) << "no storage file in the directory, so nothing is being told apart"; + const auto & lexically_first = segments.front(); + + std::filesystem::path metadata_first; + for (const auto & segment : segments) { + if (segment != directory_first && segment != lexically_first) { + metadata_first = segment; + break; + } + } + ASSERT_FALSE(metadata_first.empty()) << "three segments always leave one that is neither"; + + std::vector names{metadata_first.filename().string()}; + for (const auto & segment : segments) { + if (segment != metadata_first) { + names.push_back(segment.filename().string()); + } + } + write_metadata(split_dir, names); + + EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(split_dir.string()), metadata_first.string()) + << "the download must hand over the segment the recording names first, not the one the directory offers"; + + 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_named = std::filesystem::file_size(metadata_first); + + // 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); + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + ASSERT_TRUE(descriptors[0].x_medkit->contains("storage_files")) << "a split recording must say how many files it has"; + EXPECT_EQ((*descriptors[0].x_medkit)["storage_files"], 3) << "the count is what the bag's own metadata names"; + EXPECT_EQ(descriptors[0].size, split_total) << "a split recording is still listed at the whole recording's size"; + EXPECT_NE(descriptors[0].size, first_named) << "the segment on the wire is not the recording"; +} + +TEST_F(RosbagBagDirectoryTest, ASplitRecordingSkipsAFirstSegmentThatIsNoLongerOnDisk) { + // Quota eviction and a half-copied bag both leave metadata naming a file that + // is gone. Resolving to a path that does not exist answers 500 for a recording + // whose remaining segments are readable, so the first name that IS on disk is + // served instead. The survivor named first is again the one the directory does + // not yield first, so directory order cannot produce this answer either. + const auto split_dir = bag_dir_ / "split_first_gone"; + std::filesystem::create_directories(split_dir); + const std::filesystem::path evicted = split_dir / "recording_0.db3"; + write_file(split_dir / "recording_1.db3", std::string(24576, 'c')); + write_file(split_dir / "recording_2.db3", std::string(40960, 'd')); + write_metadata(split_dir, {"recording_1.db3"}); + ASSERT_FALSE(std::filesystem::exists(evicted)) << "the first segment has to be missing"; + + const auto directory_first = first_in_directory_order(split_dir); + ASSERT_FALSE(directory_first.empty()); + const std::filesystem::path survivor = + directory_first == split_dir / "recording_1.db3" ? split_dir / "recording_2.db3" : split_dir / "recording_1.db3"; + + write_metadata(split_dir, + {evicted.filename().string(), survivor.filename().string(), directory_first.filename().string()}); + + EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(split_dir.string()), survivor.string()) + << "a named segment that is not on disk cannot be the one served, and the directory does not get the vote"; + + const json row{{"fault_code", "SPLIT_FAULT"}, + {"recording_id", "fault_SPLIT_FAULT_1738664999001"}, + {"file_path", split_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", 99999}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + ASSERT_TRUE(descriptors[0].x_medkit->contains("storage_files")); + EXPECT_EQ((*descriptors[0].x_medkit)["storage_files"], 3) + << "the count is what the recording holds, not what survived on disk"; +} + +TEST_F(RosbagBagDirectoryTest, ASplitRecordingWithNoSegmentLeftResolvesToNothing) { + // Nothing to hand over, and the download route turns an empty resolution into + // its scoped error. The count still answers, because the metadata is readable + // and it is the recording's own record of what it held. + const auto split_dir = bag_dir_ / "split_all_gone"; + std::filesystem::create_directories(split_dir); + write_metadata(split_dir, {"recording_0.db3", "recording_1.db3"}); + + EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(split_dir.string()), ""); }); + + const json row{{"fault_code", "SPLIT_FAULT"}, + {"recording_id", "fault_SPLIT_FAULT_1738664999002"}, + {"file_path", split_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", 4242}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + ASSERT_TRUE(descriptors[0].x_medkit->contains("storage_files")); + EXPECT_EQ((*descriptors[0].x_medkit)["storage_files"], 2); + EXPECT_EQ(descriptors[0].size, 4242u) << "an unmeasurable recording keeps the figure its row carried"; +} + +// === A readable metadata is the whole answer, the directory is not consulted === +// The directory walk exists for a bag that will not say what it holds. Reaching +// it after the bag HAS said, because none of the names it gave is on disk, put a +// file the recording never named on the wire under that recording's id and under +// a storage_files the served file is not one of. What a client received was then +// neither the recording nor an error, and nothing in the response said so. + +TEST_F(RosbagBagDirectoryTest, ASplitRecordingWithNoSegmentLeftDoesNotServeAStrayBesideIt) { + // Several names, none on disk, a stray storage file beside them. The stray is + // a .db3, so the directory walk would reach it, and it is not one of the two + // files the metadata names. + const auto split_dir = bag_dir_ / "split_gone_with_stray"; + std::filesystem::create_directories(split_dir); + write_file(split_dir / "stray.db3", std::string(8192, 's')); + write_metadata(split_dir, {"recording_0.db3", "recording_1.db3"}); + ASSERT_FALSE(std::filesystem::exists(split_dir / "recording_0.db3")); + ASSERT_FALSE(std::filesystem::exists(split_dir / "recording_1.db3")); + ASSERT_EQ(first_in_directory_order(split_dir), split_dir / "stray.db3") + << "the directory walk cannot reach the stray, so nothing is being told apart"; + + EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(split_dir.string()), ""); }) + << "a file the recording never named must not be served in its place"; + + const json row{{"fault_code", "SPLIT_FAULT"}, + {"recording_id", "fault_SPLIT_FAULT_1738664999004"}, + {"file_path", split_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", 4242}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + ASSERT_TRUE(descriptors[0].x_medkit->contains("storage_files")); + EXPECT_EQ((*descriptors[0].x_medkit)["storage_files"], 2) << "the count is still what the metadata names"; + EXPECT_EQ(descriptors[0].size, 4242u) << "and the row's figure is kept, not the stray's size"; +} + +TEST_F(RosbagBagDirectoryTest, AWholeRecordingWhoseOnlyFileIsGoneDoesNotServeAStrayBesideIt) { + // One name, not on disk, a stray storage file beside it. The same rule as the + // several-name case above, on the shape that reaches a user first, because a + // recording held in one file is the normal case. + const auto gone_dir = bag_dir_ / "single_gone_with_stray"; + std::filesystem::create_directories(gone_dir); + write_file(gone_dir / "stray.db3", std::string(8192, 's')); + write_metadata(gone_dir, {"recording_0.db3"}); + ASSERT_FALSE(std::filesystem::exists(gone_dir / "recording_0.db3")); + ASSERT_EQ(first_in_directory_order(gone_dir), gone_dir / "stray.db3"); + + EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(gone_dir.string()), ""); }) + << "a file the recording never named must not be served in its place"; + + const json row{{"fault_code", "GONE_FAULT"}, + {"recording_id", "single_gone_with_stray"}, + {"file_path", gone_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", 777}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + ASSERT_TRUE(descriptors[0].x_medkit->contains("storage_files")); + EXPECT_EQ((*descriptors[0].x_medkit)["storage_files"], 1); + EXPECT_EQ(descriptors[0].size, 777u) << "the listing keeps the row's figure rather than measuring the stray"; +} + +TEST_F(RosbagBagDirectoryTest, AnEmptyStorageFileListIsTreatedAsABagThatWillNotSay) { + // A metadata that names nothing (`relative_file_paths: []`). That is a bag + // which did not answer, not a bag holding zero files: there is a storage file + // in the directory. Counting it at zero would describe the recording as empty, + // which is the value the count exists to avoid, so the field is omitted and the + // directory walk answers, exactly as for metadata that cannot be read at all. + const auto empty_list_dir = bag_dir_ / "empty_list"; + std::filesystem::create_directories(empty_list_dir); + write_file(empty_list_dir / "recording_0.db3", std::string(3072, 'e')); + write_metadata(empty_list_dir, {}); + + EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(empty_list_dir.string()), + (empty_list_dir / "recording_0.db3").string()) + << "an empty list is not an answer, so the directory is still consulted"; + EXPECT_FALSE(handlers::detail::rosbag_storage_file_count(empty_list_dir.string()).has_value()) + << "a bag that named nothing must not be counted at zero"; + + const json row{{"fault_code", "EMPTY_LIST"}, + {"recording_id", "empty_list"}, + {"file_path", empty_list_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", 555}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + EXPECT_FALSE(descriptors[0].x_medkit->contains("storage_files")) << "omitted, not zero"; +} + +TEST_F(RosbagBagDirectoryTest, ANameThatLeavesTheBagDirectoryIsSkipped) { + // A name that climbs out of the bag directory, and a name that is absolute. + // The names come out of a file on disk and are joined onto the bag path, so a + // name that climbs out of the directory, or replaces it outright by being + // absolute, would resolve to a file outside the recording and the download + // would stream it. Both are skipped, and with no other named file present that + // leaves nothing to serve. + const auto escape_dir = bag_dir_ / "escape"; + std::filesystem::create_directories(escape_dir); + const auto outside = bag_dir_ / "outside_target.db3"; + write_file(outside, std::string(6144, 'o')); + ASSERT_TRUE(std::filesystem::exists(outside)) << "the escape target has to exist, or nothing is being told apart"; + + write_metadata(escape_dir, {"../outside_target.db3"}); + EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(escape_dir.string()), ""); }) + << "a relative name that climbs out of the bag must not be served"; + + write_metadata(escape_dir, {outside.string()}); + EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(escape_dir.string()), ""); }) + << "an absolute name replaces the bag path outright and must not be served"; + + // Positive control on the same harness: the same file, named the way rosbag2 + // names one, is served. So the two refusals above are the escape being + // refused and not the resolver failing to find anything at all. + write_file(escape_dir / "recording_0.db3", std::string(1024, 'r')); + write_metadata(escape_dir, {"recording_0.db3"}); + EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(escape_dir.string()), (escape_dir / "recording_0.db3").string()); +} + +TEST_F(RosbagBagDirectoryTest, StorageFileCountIsOneForAWholeRecordingAndAbsentWhenTheBagWillNotSay) { + // One has to be stated rather than left out, because an absent field already + // means something else here: that this side could not read the recording's + // metadata at all. A client cannot tell "one file" from "unknown" if both are + // silence. + const json whole_row{{"fault_code", "MOTOR_OVERHEAT"}, + {"recording_id", bag_dir_.filename().string()}, + {"file_path", bag_dir_.string()}, + {"format", "sqlite3"}, + {"size_bytes", directory_total()}}; + + const auto whole = handlers::detail::fold_rosbag_rows_into_descriptors({whole_row}, {}); + ASSERT_EQ(whole.size(), 1u); + ASSERT_TRUE(whole[0].x_medkit.has_value()); + ASSERT_TRUE(whole[0].x_medkit->contains("storage_files")) << "a whole recording states its one file"; + EXPECT_EQ((*whole[0].x_medkit)["storage_files"], 1); + + // A bare storage file is one storage file by definition, and has no + // metadata.yaml beside it under that name to consult. + const auto bare_file = bag_dir_ / "standalone.db3"; + write_file(bare_file, std::string(2048, 'z')); + const json bare_row{{"fault_code", "BARE"}, + {"recording_id", "standalone.db3"}, + {"file_path", bare_file.string()}, + {"format", "sqlite3"}, + {"size_bytes", 1}}; + const auto bare = handlers::detail::fold_rosbag_rows_into_descriptors({bare_row}, {}); + ASSERT_EQ(bare.size(), 1u); + ASSERT_TRUE(bare[0].x_medkit.has_value()); + ASSERT_TRUE(bare[0].x_medkit->contains("storage_files")); + EXPECT_EQ((*bare[0].x_medkit)["storage_files"], 1); + + // No metadata to read: the field is omitted rather than guessed at one. The + // assertions above are the positive control for this absence - the same helper + // on the same harness does emit the field when the bag answers. + const auto silent_dir = bag_dir_ / "no_metadata"; + std::filesystem::create_directories(silent_dir); + write_file(silent_dir / "recording_0.db3", std::string(1024, 'q')); + const json silent_row{{"fault_code", "SILENT"}, + {"recording_id", "no_metadata"}, + {"file_path", silent_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", 1024}}; + const auto silent = handlers::detail::fold_rosbag_rows_into_descriptors({silent_row}, {}); + ASSERT_EQ(silent.size(), 1u); + ASSERT_TRUE(silent[0].x_medkit.has_value()); + EXPECT_FALSE(silent[0].x_medkit->contains("storage_files")) << "a bag that will not say must not be counted at one"; +} + +TEST_F(RosbagBagDirectoryTest, TheDownloadAndTheListingAgreeOnWhichSegmentIsServed) { + // download() resolves the row's file_path through + // BulkDataHandlers::resolve_rosbag_file_path and reports that file's length as + // Content-Length. The call below is that one, with that argument, so the two + // sides cannot answer differently for one recording. What is pinned here is + // the pair: the transfer is the recording's first segment while the descriptor + // keeps the whole recording's size, and the gap between them is what a client + // reads as "this is a part". + const auto split_dir = bag_dir_ / "split_agreement"; + std::filesystem::create_directories(split_dir); + write_file(split_dir / "recording_0.db3", std::string(16384, 'a')); + write_file(split_dir / "recording_1.db3", std::string(53248, 'b')); + write_metadata(split_dir, {"recording_0.db3"}); + + const auto directory_first = first_in_directory_order(split_dir); + ASSERT_FALSE(directory_first.empty()); + const std::filesystem::path metadata_first = + directory_first == split_dir / "recording_0.db3" ? split_dir / "recording_1.db3" : split_dir / "recording_0.db3"; + write_metadata(split_dir, {metadata_first.filename().string(), directory_first.filename().string()}); + + 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 std::string served_path = BulkDataHandlers::resolve_rosbag_file_path(split_dir.string()); + ASSERT_EQ(served_path, metadata_first.string()); + std::error_code ec; + const uint64_t content_length = std::filesystem::file_size(served_path, ec); + ASSERT_FALSE(static_cast(ec)); + + const json row{{"fault_code", "SPLIT_FAULT"}, + {"recording_id", "fault_SPLIT_FAULT_1738664999003"}, + {"file_path", split_dir.string()}, + {"format", "sqlite3"}, + {"size_bytes", split_total}}; + + const auto descriptors = handlers::detail::fold_rosbag_rows_into_descriptors({row}, {}); + ASSERT_EQ(descriptors.size(), 1u); + EXPECT_EQ(content_length, std::filesystem::file_size(metadata_first)) << "the transfer is that segment, whole"; + EXPECT_GT(descriptors[0].size, content_length) << "size exceeding Content-Length is how a client sees a split"; + ASSERT_TRUE(descriptors[0].x_medkit.has_value()); + ASSERT_TRUE(descriptors[0].x_medkit->contains("storage_files")); + EXPECT_EQ((*descriptors[0].x_medkit)["storage_files"], 2) << "and the count tells it how many there were"; +} + // 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 @@ -556,6 +948,10 @@ TEST_F(UnreadableBagTest, AnUnreadableBagCostsItsOwnRowAndNotTheListing) { // 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()), ""); }); + // The count reaches the filesystem on its own, before any metadata is read, to + // decide whether the path is a bare storage file. It runs on the same row of + // the same listing, so it has to decline an unreadable bag the same way. + EXPECT_NO_THROW({ EXPECT_FALSE(handlers::detail::rosbag_storage_file_count(loop_.string()).has_value()); }); } // The second shape: a directory whose mode denies everyone. This one does test @@ -583,6 +979,7 @@ TEST_F(UnreadableBagTest, AModeZeroDirectoryIsAlsoDeclinedRatherThanThrown) { EXPECT_NO_THROW({ EXPECT_FALSE(handlers::detail::rosbag_served_bytes(locked.string()).has_value()); }); EXPECT_NO_THROW({ EXPECT_EQ(BulkDataHandlers::resolve_rosbag_file_path(locked.string()), ""); }); + EXPECT_NO_THROW({ EXPECT_FALSE(handlers::detail::rosbag_storage_file_count(locked.string()).has_value()); }); std::filesystem::permissions(locked, std::filesystem::perms::owner_all, ec); }