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

Structure member optionality of shapes reachable via operation outputs in IDL v2 in servers #1479

Closed
david-perez opened this issue Oct 31, 2022 · 3 comments

Comments

@david-perez
Copy link
Contributor

david-perez commented Oct 31, 2022

The spec says that a server (i.e. an authoritative model consumer) should render a structure member shape with @default as

Present

(unconditionally). My reading of that is that the bar member of the Foo structure in this model:

operation MyOperation {
    output: Foo
}

structure Foo {
    @default("hello")
    bar: String
}

should be rendered in Rust as not optional, i.e. a plain std::string::String:

struct Foo {
    bar: String
}

But I don't understand the purpose of @default then in this particular example. The service owner still has to always provide a value for bar when returning a response; the default value "hello" is never used.

In the same way the spec makes provisions for client structure members when talking about operation inputs, shouldn't those provisions be symmetric for servers? So, in particular, these make sense for server SDKs:

  1. Everyting that is reachable via an operation input: every member in an @input shape with @default should be rendered as present (T): the server SDK will fill in the member using the default value if it receives a request that does not set it.
  2. Everyting that is reachable via an operation output: every member in an @output shape with @default should be rendered as optional (Option<T>): the server SDK will fill in the member using the default value prior to sending a response if the service owner did not set it.
@mtdowling
Copy link
Member

Does this need two separate issues? Seems like a very minor change to what was asked in #1478

@david-perez
Copy link
Contributor Author

While the illustrating example is identical, I opened this issue because I was surprised about the asymmetry in the provisions for optionality in server SDKs. For example, one would intuitively think that there should be an @output row in that table affecting servers: after all, an operation input for a client plays the same role as an operation output for a service owner.

But I think the asymmetry is warranted, since those provisions are there to preserve backwards-compatibility in clients, something that an authoritative model consumer does not need to worry about.

So, to answer my own questions:

The service owner still has to always provide a value for bar when returning a response; the default value "hello" is never used.

Not in the case where the service owner is using a builder; Foo::builder().build() will set bar to String::from("hello"). In the case where the service owner is directly instantiating the struct, we gain nothing by rendering bar with type Option<String>.

every member in an @input shape with @default should be rendered as present (T): the server SDK will fill in the member using the default value if it receives a request that does not set it.

This is already contemplated in the spec. For a server SDK, @default implies the member is present, regardless of @input/@output.

every member in an @output shape with @default should be rendered as optional (Option<T>): the server SDK will fill in the member using the default value prior to sending a response if the service owner did not set it.

The server SDK can do this in the builder regardless of whether the member is optional or not. We gain nothing by rendering as Option<T>, so best to keep it as T.


So I'm happy with that table for server SDKs; feel free to close the issue if you agree with my reasoning.

@mtdowling
Copy link
Member

mtdowling commented Nov 7, 2022

Looks right to me!

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

No branches or pull requests

2 participants