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

RFC: feat: flatten request #1159

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

polachok
Copy link
Contributor

I would like to receive request as a single struct instead of a parameter list, because I'd like to do validation on it and pass it further, but it's annoying to write let req = Request { a, b, c, ... } every time.

So I propose a new attribute: flatten (like serde(flatten)).

#[method(name = "echo", param_kind = map, flatten)]
async fn echo(&self, req: EchoRequest) -> Result<(), Error>

makes Request consume entire params field.

@polachok polachok requested a review from a team as a code owner July 27, 2023 16:23
@niklasad1
Copy link
Member

Hey @polachok

I don't really follow why the Request is the annoyance because your fix is in the "proc macro API" and that shouldn't expose the request just the params but most likely you are doing something more complicated that I don't follow...

Can you elaborate with an example what you want to do?

@polachok
Copy link
Contributor Author

Imagine you have many params in api call, so the request looks like this:

{ 
"jsonrpc": "2.0", 
"id": "12345", 
"method": "doTheThing", 
"params": { 
 "first": "AAA", 
 "second": 5884105, 
 "third": "2000", 
 "fourth": 1, 
 "fifth": "BBB",
 "sixth": "CCC", 
 "seventh": "SSS", 
 } 
}

And the server method would look like

#[method(name = "doTheThing", param_kind = map)]
fn do_the_thing(&self, first: String, second: u64, third: String, fourth: i32, fifth: String, sixth: String, seventh: String) -> ... {
   let thing_request = ThingRequest { first, second, third, fourth, ... }; // kinda annoying!
   thing_request.validate()?;
   do_something(thing_request);
   ...
}

I would like to write this instead:

#[method(name = "doTheThing", param_kind = map, flatten)]
fn do_the_thing(&self, thing_request: ThingRequest) -> ... {
   thing_request.validate()?;
   do_something(thing_request);
   ...
}

@jsdw
Copy link
Collaborator

jsdw commented Jul 31, 2023

I think with the current logic, having

#[method(name = "echo", param_kind = map, flatten)]
async fn echo(&self, req: EchoRequest) -> Result<(), Error>

Would mean that every parameter is "flattened" into the params object (or really, un-flattened; flatten is really talking about how to serialize nested obejcts into a flattened object).

So eg:

struct Foo {a: u8, b: u8}
struct Bar{c: bool, d: String}

#[method(name = "echo", param_kind = map, flatten)]
async fn echo(&self, foo: Foo, bar: Bar) -> Result<(), Error>

I think this would work too and would expect params like { "a": 1, "b": 2, "c": true, "d": "hello" }.

Probably not a big deal though; normally you'd only flatten one param anyway (I don't see why you'd want a mixture). possibnly we could deliberately restrict to one param for clarity.

This would also allow eg HashMap<String, u8> to be a parameter that would decode any valid numbers; I wonder if that poses any sort of issue or whether it's just a nice thing that's enabled!

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.

3 participants