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

[Bug]: Weird behavior for spread model with additional properties #4757

Open
4 tasks done
tadelesh opened this issue Oct 16, 2024 · 3 comments
Open
4 tasks done

[Bug]: Weird behavior for spread model with additional properties #4757

tadelesh opened this issue Oct 16, 2024 · 3 comments
Assignees
Labels
bug Something isn't working lib:http

Comments

@tadelesh
Copy link
Member

Describe the bug

For this spec which is simplified from a service definition here, there is a usage of spread model with additional properties (though i think it should be banned). Also, the http body is composed from both the spread thing and azure core traits thing, then the weird thing happened, the http body parameter type will have a query parameter from the trait, which will not happen is the spread model has no additional properties.
Image
I'm wondering if it is a bug for body resolving. But I think a good solution is not allow to spread model with additional properties.

Reproduction

Compile with the playground or the spec provided.

Checklist

@timotheeguerin
Copy link
Member

timotheeguerin commented Oct 16, 2024

But I think a good solution is not allow to spread model with additional properties.

This is allowed by design, something we explicitly designed a few month ago, i am not sure I see the issue in teh simplified spec, everthing looks correct no?

@timotheeguerin timotheeguerin added the needs-info Mark an issue that needs reply from the author or it will be closed automatically label Oct 16, 2024
@tadelesh
Copy link
Member Author

But I think a good solution is not allow to spread model with additional properties.

This is allowed by design, something we explicitly designed a few month ago, i am not sure I see the issue in teh simplified spec, everthing looks correct no?

if i spread a model A with additional properties to model B, then the model B will have additional properties? what if the spread happen in operation parameter?

the problem is showed in the screenshot, the body type of http operation will have a property of api version query parameter. if remove the ...Record<unknown>, the query will not in the body type.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the needs-info Mark an issue that needs reply from the author or it will be closed automatically label Oct 17, 2024
@timotheeguerin
Copy link
Member

oh I see, yeah seems like we do have a special case that bypass the filterModelProperties part if it has an indexer in the body resolution.

So 2 things:

  1. we probably have a fix to do in http library
  2. We are not filtering properties for any nested types today. This is something the http typekits would I think improve on. This means if you are having an issue with this pattern you would also not exclude nested query params most likely
op getEmbeddings(@bodyIgnore nested: {@query foo: string}, name: string): void;

playground

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib:http
Projects
None yet
Development

No branches or pull requests

2 participants