Skip to content

Commit 8f806de

Browse files
authored
Merge DroppableEncoder and Encoder into a single facility (#11446)
Our RRD encoding facilities are currently implemented by two distinct types: `DroppableEncoder` & `Encoder`. This makes understanding how to use the API unnecessarily painful. Turns out the only reason things are done this way is because of an implementation detail related to the `Drop` implementation. Instead of passing off that complexity to end-users of the API, we now contain it to the implementation instead. In addition to that, the `AlreadyFinished` logic was broken, so fix too while we're at it. --- This PR is part of an upcoming series of PRs to pay off organic growth debt in our encoding/decoding stack.
1 parent a415cf8 commit 8f806de

File tree

10 files changed

+122
-138
lines changed

10 files changed

+122
-138
lines changed

crates/store/re_data_loader/src/loader_rrd.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ impl RetryableFileReader {
329329
#[cfg(test)]
330330
mod tests {
331331
use re_chunk::RowId;
332-
use re_log_encoding::encoder::DroppableEncoder;
332+
use re_log_encoding::encoder::Encoder;
333333
use re_log_types::{LogMsg, SetStoreInfo, StoreId, StoreInfo, StoreKind, StoreSource};
334334

335335
use super::*;
@@ -358,7 +358,7 @@ mod tests {
358358
.open(rrd_file_path.to_str().unwrap())
359359
.unwrap();
360360

361-
let mut encoder = DroppableEncoder::new(
361+
let mut encoder = Encoder::new(
362362
re_build_info::CrateVersion::LOCAL,
363363
re_log_encoding::EncodingOptions::PROTOBUF_UNCOMPRESSED,
364364
rrd_file,

crates/store/re_log_encoding/src/decoder/mod.rs

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ mod tests {
485485
use super::*;
486486
use crate::Compression;
487487
use crate::codec::arrow::encode_arrow;
488-
use crate::encoder::DroppableEncoder;
488+
use crate::encoder::Encoder;
489489

490490
pub fn fake_log_messages() -> Vec<LogMsg> {
491491
let store_id = StoreId::random(StoreKind::Blueprint, "test_app");
@@ -676,7 +676,7 @@ mod tests {
676676
for options in options {
677677
let mut file = vec![];
678678

679-
let mut encoder = DroppableEncoder::new(rrd_version, options, &mut file).unwrap();
679+
let mut encoder = Encoder::new(rrd_version, options, &mut file).unwrap();
680680
for message in messages.clone() {
681681
encoder
682682
.append_proto(message)
@@ -735,7 +735,7 @@ mod tests {
735735
for options in options {
736736
let mut file = vec![];
737737

738-
let mut encoder = DroppableEncoder::new(rrd_version, options, &mut file).unwrap();
738+
let mut encoder = Encoder::new(rrd_version, options, &mut file).unwrap();
739739
for message in out_of_order_messages.clone() {
740740
encoder
741741
.append_proto(message)
@@ -824,23 +824,28 @@ mod tests {
824824
let messages = fake_log_messages();
825825

826826
// (2 encoders as each encoder writes a file header)
827-
let writer = std::io::Cursor::new(&mut data);
828-
let mut encoder1 =
829-
crate::encoder::Encoder::new(CrateVersion::LOCAL, options, writer).unwrap();
830-
for message in &messages {
831-
encoder1.append(message).unwrap();
827+
{
828+
let writer = std::io::Cursor::new(&mut data);
829+
let mut encoder1 =
830+
crate::encoder::Encoder::new(CrateVersion::LOCAL, options, writer).unwrap();
831+
for message in &messages {
832+
encoder1.append(message).unwrap();
833+
}
834+
encoder1.finish().unwrap();
832835
}
833-
encoder1.finish().unwrap();
834836

835837
let written = data.len() as u64;
836-
let mut writer = std::io::Cursor::new(&mut data);
837-
writer.set_position(written);
838-
let mut encoder2 =
839-
crate::encoder::Encoder::new(CrateVersion::LOCAL, options, writer).unwrap();
840-
for message in &messages {
841-
encoder2.append(message).unwrap();
838+
839+
{
840+
let mut writer = std::io::Cursor::new(&mut data);
841+
writer.set_position(written);
842+
let mut encoder2 =
843+
crate::encoder::Encoder::new(CrateVersion::LOCAL, options, writer).unwrap();
844+
for message in &messages {
845+
encoder2.append(message).unwrap();
846+
}
847+
encoder2.finish().unwrap();
842848
}
843-
encoder2.finish().unwrap();
844849

845850
let decoder =
846851
Decoder::new_concatenated(std::io::BufReader::new(data.as_slice())).unwrap();

crates/store/re_log_encoding/src/decoder/stream.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,15 +330,14 @@ mod tests {
330330
fn test_data(options: EncodingOptions, n: usize) -> (Vec<LogMsg>, Vec<u8>) {
331331
let messages: Vec<_> = (0..n).map(|_| fake_log_msg()).collect();
332332

333-
let mut buffer = Vec::new();
334-
let mut encoder = Encoder::new(CrateVersion::LOCAL, options, &mut buffer).unwrap();
333+
let mut encoder = Encoder::new(CrateVersion::LOCAL, options, vec![]).unwrap();
335334
for message in &messages {
336335
encoder.append(message).unwrap();
337336
}
338337

339338
encoder.finish().unwrap();
340339

341-
(messages, buffer)
340+
(messages, encoder.into_inner().unwrap())
342341
}
343342

344343
macro_rules! assert_message_ok {

0 commit comments

Comments
 (0)