Skip to content
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

[fastx client] Create a clean object reading interface (Part 4) #392

Merged
merged 7 commits into from
Feb 9, 2022

Conversation

gdanezis
Copy link
Collaborator

@gdanezis gdanezis commented Feb 8, 2022

  • Introduce a ObjectRead structure with a clear Exists, NotExists, Deleted set of enums.
  • Change get_object_info on client to just take object ID and return the last version as an ObjectRead
  • Clean up tests to not rely on old get_object_info semantics
  • In the authority aggregator removed old get_object_info logic

@gdanezis gdanezis changed the title [fastx client] Create a clean object reading interface [fastx client] Create a clean object reading interface (Part 4) Feb 8, 2022
@gdanezis gdanezis changed the title [fastx client] Create a clean object reading interface (Part 4) [fastx client] Create a clean object reading interface (Part 4) (WIP) Feb 8, 2022
@gdanezis gdanezis changed the title [fastx client] Create a clean object reading interface (Part 4) (WIP) [fastx client] Create a clean object reading interface (Part 4) Feb 8, 2022
Copy link
Contributor

@lxfind lxfind left a comment

Choose a reason for hiding this comment

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

Approve with some suggestions.

@@ -48,6 +49,30 @@ pub struct ClientState<AuthorityAPI> {
store: ClientStore,
}

pub enum ObjectRead {
Copy link
Contributor

Choose a reason for hiding this comment

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

ObjectReadResult is probably a better name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In Rust lingo Result usually suggests its a Result< something, something Error>, so I would stay away from that word.

impl ObjectRead {
pub fn object(&self) -> Result<&Object, FastPayError> {
match &self {
ObjectRead::Deleted(oref) => Err(FastPayError::ObjectDeleted { object_ref: *oref }),
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use Self whenever ObjectRead is needed, so that if we ever rename the struct you don't need to change them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great suggestion! Then we can work out what to call it.

@gdanezis gdanezis force-pushed the check-authority-responses branch from a50c41e to 2a7572a Compare February 9, 2022 11:15
@gdanezis gdanezis changed the base branch from check-authority-responses to main February 9, 2022 11:23
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.

2 participants