Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ellie committed Jan 5, 2024
1 parent 28a804c commit 0e8bd74
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 31 deletions.
22 changes: 11 additions & 11 deletions atuin-client/src/history/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl HistoryRecord {
Ok(DecryptedData(output))
}

pub fn deserialize(bytes: &[u8]) -> Result<Self> {
pub fn deserialize(bytes: &[u8], version: &str) -> Result<Self> {
use rmp::decode;

fn error_report<E: std::fmt::Debug>(err: E) -> eyre::Report {
Expand All @@ -76,7 +76,7 @@ impl HistoryRecord {
// written by write_bin above
let _ = decode::read_bin_len(&mut bytes).map_err(error_report)?;

let record = History::deserialize(bytes.remaining_slice(), HISTORY_VERSION)?;
let record = History::deserialize(bytes.remaining_slice(), version)?;

Ok(HistoryRecord::Create(record))
}
Expand Down Expand Up @@ -153,7 +153,7 @@ impl HistoryStore {
mod tests {
use time::macros::datetime;

use crate::history::store::HistoryRecord;
use crate::history::{store::HistoryRecord, HISTORY_VERSION};

use super::History;

Expand Down Expand Up @@ -187,13 +187,13 @@ mod tests {
let serialized = record.serialize().expect("failed to serialize history");
assert_eq!(serialized.0, bytes);

let deserialized =
HistoryRecord::deserialize(&serialized.0).expect("failed to deserialize HistoryRecord");
let deserialized = HistoryRecord::deserialize(&serialized.0, HISTORY_VERSION)
.expect("failed to deserialize HistoryRecord");
assert_eq!(deserialized, record);

// check the snapshot too
let deserialized =
HistoryRecord::deserialize(&bytes).expect("failed to deserialize HistoryRecord");
let deserialized = HistoryRecord::deserialize(&bytes, HISTORY_VERSION)
.expect("failed to deserialize HistoryRecord");
assert_eq!(deserialized, record);
}

Expand All @@ -208,12 +208,12 @@ mod tests {
let serialized = record.serialize().expect("failed to serialize history");
assert_eq!(serialized.0, bytes);

let deserialized =
HistoryRecord::deserialize(&serialized.0).expect("failed to deserialize HistoryRecord");
let deserialized = HistoryRecord::deserialize(&serialized.0, HISTORY_VERSION)
.expect("failed to deserialize HistoryRecord");
assert_eq!(deserialized, record);

let deserialized =
HistoryRecord::deserialize(&bytes).expect("failed to deserialize HistoryRecord");
let deserialized = HistoryRecord::deserialize(&bytes, HISTORY_VERSION)
.expect("failed to deserialize HistoryRecord");
assert_eq!(deserialized, record);
}
}
16 changes: 8 additions & 8 deletions atuin-client/src/record/sqlite_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,14 @@ impl Store for SqliteStore {
self.idx(host, tag, 0).await
}

async fn len(&self, host: HostId, tag: &str) -> Result<Option<u64>> {
async fn len(&self, host: HostId, tag: &str) -> Result<u64> {
let last = self.last(host, tag).await?;

if let Some(last) = last {
return Ok(Some(last.idx + 1));
return Ok(last.idx + 1);
}

return Ok(None);
return Ok(0);
}

async fn next(
Expand Down Expand Up @@ -336,7 +336,7 @@ mod tests {
.await
.expect("failed to get store len");

assert_eq!(len, Some(1), "expected length of 1 after insert");
assert_eq!(len, 1, "expected length of 1 after insert");
}

#[tokio::test]
Expand All @@ -355,8 +355,8 @@ mod tests {
let first_len = db.len(first.host.id, first.tag.as_str()).await.unwrap();
let second_len = db.len(second.host.id, second.tag.as_str()).await.unwrap();

assert_eq!(first_len, Some(1), "expected length of 1 after insert");
assert_eq!(second_len, Some(1), "expected length of 1 after insert");
assert_eq!(first_len, 1, "expected length of 1 after insert");
assert_eq!(second_len, 1, "expected length of 1 after insert");
}

#[tokio::test]
Expand All @@ -373,7 +373,7 @@ mod tests {

assert_eq!(
db.len(tail.host.id, tail.tag.as_str()).await.unwrap(),
Some(100),
100,
"failed to insert 100 records"
);
}
Expand All @@ -396,7 +396,7 @@ mod tests {

assert_eq!(
db.len(tail.host.id, tail.tag.as_str()).await.unwrap(),
Some(10000),
10000,
"failed to insert 10k records"
);
}
Expand Down
5 changes: 2 additions & 3 deletions atuin-client/src/record/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use async_trait::async_trait;
use eyre::Result;

use atuin_common::record::{EncryptedData, HostId, Record, RecordId, RecordIdx, RecordStatus};

/// A record store stores records
/// In more detail - we tend to need to process this into _another_ format to actually query it.
/// As is, the record store is intended as the source of truth for arbitratry data, which could
Expand All @@ -21,12 +20,12 @@ pub trait Store {
) -> Result<()>;

async fn get(&self, id: RecordId) -> Result<Record<EncryptedData>>;
async fn len(&self, host: HostId, tag: &str) -> Result<Option<u64>>;
async fn len(&self, host: HostId, tag: &str) -> Result<u64>;

async fn last(&self, host: HostId, tag: &str) -> Result<Option<Record<EncryptedData>>>;
async fn first(&self, host: HostId, tag: &str) -> Result<Option<Record<EncryptedData>>>;

/// Get the record that follows this record
/// Get the next `limit` records, after and including the given index
async fn next(
&self,
host: HostId,
Expand Down
11 changes: 2 additions & 9 deletions atuin-common/src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub struct EncryptedData {
pub content_encryption_key: String,
}

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, PartialOrd, Ord, Eq)]
pub struct Diff {
pub host: HostId,
pub tag: String,
Expand Down Expand Up @@ -194,14 +194,7 @@ impl RecordStatus {
}

// Stability is a nice property to have
ret.sort_by(|a, b| {
(a.host, a.tag.clone(), a.local, a.remote).cmp(&(
b.host,
b.tag.clone(),
b.local,
b.remote,
))
});
ret.sort();
ret
}
}
Expand Down

0 comments on commit 0e8bd74

Please sign in to comment.