Skip to content

Store the timeline in the SledStore #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

DevinR528
Copy link
Contributor

May resolve #138

So this is SUPER work in progress just wanted to have a place to look at code and try things out...

Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll discuss this further on the issue. Just some obvious nits here.

@@ -529,6 +527,19 @@ impl BaseClient {
}
}

// TODO:
// We wait until after the event has been decrypted?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry what are you asking here? The block above already makes sure that all events that can be decrypted during this sync are decrypted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was just making sure that we wanted the events decrypted I figured since the Store can be encrypted that this is Ok but I just wanted to make sure.

resp: &api::message::get_message_events::Response,
) -> Result<()> {
let mut changes = StateChanges::default();
changes.handle_messages_response(room_id, resp, dir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method will need to decrypt the events as well, but that can be left for later on.


// A key consists of `RoomId`
let mut key = rid.as_bytes().to_vec();
key.push(0xff);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not do this, we have the EncodeKey trait for this. Every key or tuple of keys should go through an encode() call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea that was mostly just me working through the larger problem. I was planning on adding EncodeKey trait for whatever tuples we need e.g. ((&str, u128, whatever)). Sound good?

ret?;

for (rid, room) in &changes.sync_timeline {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit, let's use room_id or (room, room_events) like the rest of the code here.

@gnunicorn
Copy link
Contributor

Closing in favor of #486 .

@gnunicorn gnunicorn closed this Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storing timelines in the state store
3 participants