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

Add GraphQL client API #119

Merged
merged 21 commits into from
May 19, 2022
Merged

Add GraphQL client API #119

merged 21 commits into from
May 19, 2022

Conversation

cafca
Copy link
Member

@cafca cafca commented May 16, 2022

Replaces the "ping" placeholder with a GraphQL API for p2panda clients.

  • Add a query endpoint for next entry arguments
  • Add tests
  • Move request/response structs from rpc module and finally delete that
  • Interface with storage provider
  • Modules for serialising LogId and SeqNum as String

#60

Next

  • Add mutation for publishing entries
  • Add query for requesting documents and/or schemas

📋 Checklist

  • Add tests that cover your changes
  • Add this PR to the Unreleased section in CHANGELOG.md
  • Link this PR to any issues it closes
  • New files contain a SPDX license header

@cafca cafca requested a review from sandreae May 16, 2022 15:50
@cafca cafca marked this pull request as ready for review May 16, 2022 15:50
Copy link
Member

@sandreae sandreae left a comment

Choose a reason for hiding this comment

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

Hey, thanks for this, all the changes look good! However I think it makes sense to base this PR off the development branch. As there have been/will be a series of breaking changes (removing JSON rpc etc...) we are merging into there for now. In development you already have access to the initial storage provider implementation, so you would be able to use next_entry_args() and publish_entry() and all other existing higher level storage methods there.

@cafca cafca marked this pull request as draft May 17, 2022 15:50
@cafca cafca changed the base branch from main to development May 17, 2022 15:50
@cafca cafca marked this pull request as ready for review May 18, 2022 11:11
@cafca cafca requested a review from sandreae May 18, 2022 11:11
@cafca cafca marked this pull request as draft May 18, 2022 11:24
@cafca
Copy link
Member Author

cafca commented May 18, 2022

Oops, forgot something

@cafca
Copy link
Member Author

cafca commented May 18, 2022

I added a utility in u64_string.rs: it let's you do something like this

pub struct EntryArgsResponse {
    #[serde(with = "log_id_string_serialisation")]
    pub log_id: LogId,

    #[serde(with = "seq_num_string_serialisation")]
    pub seq_num: SeqNum,

    pub backlink: Option<Hash>,

    pub skiplink: Option<Hash>,
}

and then log_id and seq_num are serialised as string values without having to change their type on this struct or having to implement a custom serialiser. I am not sure whether this will be helpful outside of testing, maybe it should also go into p2panda-rs. What do you think @sandreae ?

@cafca cafca marked this pull request as ready for review May 18, 2022 14:07
Copy link
Member

@sandreae sandreae left a comment

Choose a reason for hiding this comment

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

This all looks good! Great to have the client GraphQL being introduced and finally completely removing JSON RPC.

Interesting to see what we can do with the string serialisation definitions too 👍

Comment on lines +7 to +11
/// Serialise log id as strings.
///
/// To be used as a parameter for Serde's `with` field attribute.
#[allow(dead_code)]
pub mod log_id_string_serialisation {
Copy link
Member

Choose a reason for hiding this comment

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

This is very cool! It's similar to what I was trying to do in the context of storing and retrieving p2panda types from the db where they are mostly strings: #89

I prefer your approach as it seems more generally useful. I don't know if it is a drop-in solution to for the db stuff though. Definitely let's keep an eye on it and see if we want to implement it in p2panda-rs and then try to use for the db models.

@sandreae sandreae merged commit c1b75d3 into development May 19, 2022
@sandreae sandreae deleted the graphql-client-api branch May 19, 2022 04:57
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