-
Notifications
You must be signed in to change notification settings - Fork 3
Initial design for Post Request scaffolding #2
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
pub struct PostsRequestBuilder {} | ||
|
||
impl PostsRequestBuilder { | ||
pub fn list(&self, params: Option<PostsListParams>) -> PostsRequest { | ||
todo!() | ||
} | ||
|
||
pub fn create(&self, params: Option<PostsCreateParams>) -> PostsRequest { | ||
todo!() | ||
} | ||
|
||
pub fn retrieve(&self, post_id: u32, params: Option<PostsRetrieveParams>) -> PostsRequest { | ||
todo!() | ||
} | ||
|
||
pub fn update(&self, post_id: u32, params: Option<PostsUpdateParams>) -> PostsRequest { | ||
todo!() | ||
} | ||
|
||
pub fn delete(&self, post_id: u32, params: Option<PostsDeleteParams>) -> PostsRequest { | ||
todo!() | ||
} | ||
} | ||
|
||
pub struct PostsListParams { | ||
pub page: Option<u32>, | ||
pub per_page: Option<u32>, | ||
} | ||
|
||
pub struct PostsCreateParams { | ||
pub title: Option<String>, | ||
pub content: Option<String>, | ||
} | ||
|
||
pub struct PostsRetrieveParams { | ||
pub password: Option<String>, | ||
} | ||
|
||
pub struct PostsUpdateParams { | ||
pub title: Option<String>, | ||
pub content: Option<String>, | ||
} | ||
|
||
pub struct PostsDeleteParams { | ||
pub force: Option<bool>, | ||
} | ||
|
||
pub struct PostsRequest { | ||
pub endpoint: String, | ||
pub params: Option<String>, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,62 @@ | ||
// These are test functions that we are keeping while we figure out the design in case | ||
// we need them for further testing. | ||
namespace wordpress_api { | ||
i32 add_custom(i32 a, i32 b); | ||
string combine_strings(string a, string b); | ||
void panic_from_rust(); | ||
}; | ||
|
||
interface RequestBuilder { | ||
PostsRequestBuilder posts(); | ||
}; | ||
|
||
// https://developer.wordpress.org/rest-api/reference/posts/ | ||
// TODO: The schema and some of the action arguments for `/posts` endpoint has the notion of `context`. | ||
// This is an `enum` value, but it can only contain partial values per field. | ||
// This is an important design element to get right, because it's a common pattern for the API. | ||
// | ||
// IMPORTANT: This design does not include error handling yet! | ||
interface PostsRequestBuilder { | ||
PostsRequest list(PostsListParams? params); | ||
PostsRequest create(PostsCreateParams? params); | ||
PostsRequest retrieve(u32 post_id, PostsRetrieveParams? params); | ||
PostsRequest update(u32 post_id, PostsUpdateParams? params); | ||
PostsRequest delete(u32 post_id, PostsDeleteParams? params); | ||
}; | ||
|
||
// If we can represent a trait relationship in UDL, we could use a common Request trait | ||
// However, we'd still want a specific `PostsRequest` type so that we can have a strongly typed | ||
// return value for it. | ||
// | ||
// The reason we can use a `PostsRequest` type and not need a more specific type such as | ||
// `PostsListRequest` is because the API documentation states that the return value for | ||
// all `/posts` requests have the same schema: https://developer.wordpress.org/rest-api/reference/posts/#schema | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, this isn't quite true – the schema depends on the context, so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's the case regardless of the action type (
That's where I was going from. However, the I've been thinking about this after opening the PR, and regardless of the result of this discussion, we are probably going to end up with specific request types. |
||
// If this is not the case, we'll need more specific types. | ||
dictionary PostsRequest { | ||
string endpoint; | ||
string? params; | ||
}; | ||
|
||
// We should check if it's possible to use the same params for update & create | ||
dictionary PostsCreateParams { | ||
string? title; | ||
string? content; | ||
}; | ||
|
||
dictionary PostsListParams { | ||
u32? page; | ||
u32? per_page; | ||
}; | ||
|
||
dictionary PostsRetrieveParams { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had a discussion about this on Zoom today, but just for completeness sake, the reason I used |
||
string? password; | ||
}; | ||
|
||
dictionary PostsUpdateParams { | ||
string? title; | ||
string? content; | ||
}; | ||
|
||
dictionary PostsDeleteParams { | ||
boolean? force; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need an HTTP method member here too.
I assume we'll give this to a higher-level
RestApi
object that will hold the hostname, http protocol, etcThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap, lots of fields are missing from this.