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

Provide a way to know which model one property is spread from #1516

Closed
chunyu3 opened this issue Jan 9, 2023 · 6 comments
Closed

Provide a way to know which model one property is spread from #1516

chunyu3 opened this issue Jan 9, 2023 · 6 comments
Assignees

Comments

@chunyu3
Copy link
Contributor

chunyu3 commented Jan 9, 2023

In CADL, the spread alias/model will act as operation parameter. CodeGen should flattern parameters defined with alias in CADL.
It will contain following scenario:

model Foo {
  name: string;
  age: int32;
}
alias FooAlias = {
 @path id: string;
 @query top: int32
  ...Foo
};

op spreadWithModel(...Foo): void
op spreadWithAlias(...FooAlias): void

We need to generate code:

void spreadWithModel(Foo foo)
void spreadWithAlias(string id, int32 top, Foo foo)

When cadl compler parse above operations (spreadWithModel and speradWithAlias), it will directly spread the model and provide an anomous model with the two properties name and age as body type. In that way, it looses the information that the two properties are spread from model Foo.
We need to know that the name and age are spread by model Foo, and then use the model Foo as the operation parameter.

Can cadl provide an api like getSpreadSource(ModelProperty) or metadata in the ModelProperty to identify which model the property is spread from?

@ghost ghost added Needs Triage labels Jan 9, 2023
@chunyu3
Copy link
Contributor Author

chunyu3 commented Jan 9, 2023

Another special case is spreading a model with multiple target (as following) as parameter.

model Foo {
  @path id: string;
  name: string;
  age: int32;
}

op spreadWithModel(...Foo)

expected to generate client library:

void spreadWithModel(Foo foo)

The cadl compile parse this operation with one path parameter id and another body parameter with body type {name: string, age: int32}.

So we need to know that the three property id, name, age are spread from model Foo, and then use model Foo as the parameter of the generated sdk method.

@nguerrera
Copy link
Contributor

ModelProperty does have sourceProperty that indicates where a property was spread from.

There is also a getEffectiveModelType that can tell you if an entire anonymous model is equivalent to a named model from which all its properties were spread. There is also a wrapper around this in MetadataInfo.getEffectivePayloadType, which is what the OpenAPI emitter uses and ideally what other emitters would use for this purpose. See #1083 which tracks an end-to-end document/sample I'm working on to demonstrate this and related things. There are more notes on the issue there in the meantime until this document is checked in.

@MaryGao
Copy link
Member

MaryGao commented Jan 11, 2023

@nguerrera I have one follow-up question, do you think the return value of getEffectiveModelType would be the same in below two cases? Can we somehow get the alias name FooAlias in case 1?

// ------------- case 1: with alias -----------
alias FooAlias = {
 @path id: string;
 @query top: int32
  name: string;
};
op spreadWithAlias(...FooAlias): void


// ------------- case 2: without alias -----------
op spreadWithAlias(...{
 @path id: string;
 @query top: int32
  name: string;
}): void

@nguerrera
Copy link
Contributor

Case 2 is not currently valid. There must be an identifier to the right of ... currently. However, if it were valid, the expectation would be that these two cases are fully equivalent and I would expct to see an emitter emit the same thing for both.

The purpose of an alias is to give a Cadl dev-time only name to something that gets erased when we produce the type graph that emitters process. So there is not a way to get the name FooAlias (without digging into the syntax tree, which emitters should not do). If FooAlias were a name that emitters should preserve, then the Cadl author should not be using alias to define it.

@chunyu3
Copy link
Contributor Author

chunyu3 commented Jan 12, 2023

Case 2 actually is to spread an anonymous model.

@nguerrera
Copy link
Contributor

Case 2 won't parse. If we ever did allow case 2 to parse and gave it meaning, then it would become equivalent to Case 1 as that is the only sensible meaning. Case 1 spreads an anonymous model. The alias name is not a model name.

getEffectiveModelType will never find an alias name. It uses the type graph from which all alias names have been erased. In general, the design is that alias names should never survive into emitter output.

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

5 participants