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.

Sign up for GitHub

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

Jump to bottom

Set request body to optional when all properties are optional #582

Open
andrueastman opened this issue Sep 20, 2024 · 5 comments · May be fixed by #591
Open

Set request body to optional when all properties are optional #582

andrueastman opened this issue Sep 20, 2024 · 5 comments · May be fixed by #591
Assignees
@irvinesunday
Labels
priority:p1 High priority but not blocking. Causes major but not critical loss of functionality SLA <=7days

Comments

@andrueastman
Copy link
Member

andrueastman commented Sep 20, 2024 •
edited
Loading

Related to microsoftgraph/msgraph-sdk-java#2163 and microsoftgraph/msgraph-sdk-dotnet#2671 (comment)

In a scenario where all the properties in the request body are optional such as an odata action with all parameters being optional, we should set the requestBody.required = false in the generated openApi description otherwise explictly set it to true.

https://spec.openapis.org/oas/v3.0.3.html#fixed-fields-10

This will unlock the generation of request body parameters in the SDKs as optional parameters in the builders to prevent breaking changes in scenarios where optional parametes are added.
@andrueastman andrueastman mentioned this issue Sep 20, 2024
Closed
@andrueastman andrueastman added the priority:p1 High priority but not blocking. Causes major but not critical loss of functionality SLA <=7days label Sep 20, 2024
@Ndiritu Ndiritu mentioned this issue Sep 23, 2024
Closed
@andrueastman andrueastman mentioned this issue Sep 25, 2024
Merged
@andrueastman
Copy link
Member Author

Once this is fixed and released, we should revert this PR.
microsoftgraph/msgraph-metadata#697

@irvinesunday irvinesunday self-assigned this Sep 30, 2024
@andrueastman
Copy link
Member Author

Updated the description here. Should be Odata action not function. Functions do not have request bodies as the parameters are in the path.

@irvinesunday
Copy link
Contributor

Also, point of note is that, if requestBody.required = false the OAS does not emit the required property in the requestBody schema.

@andrueastman
Copy link
Member Author

According to the spec.

Determines if the request body is required in the request. Defaults to false.

So what we should do is set it to true when required parameters are present...

@irvinesunday irvinesunday linked a pull request Oct 2, 2024 that will close this issue
Draft
@irvinesunday
Copy link
Contributor

Trying to retrieve the annotations in-lined in the Action parameters fails to retrieve the Org.OData.Core.V1.OptionalParameter annotation using the extensions from the Edm lib.

var annotations =  model.FindVocabularyAnnotationsIncludingInheritedAnnotations(parameter);
<Parameter Name="transitive" Type="Edm.Boolean" Nullable="false">
  <Annotation Term="Org.OData.Core.V1.OptionalParameter" />
</Parameter>

Checking with the OData team to check for its support.

This was referenced Oct 15, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@irvinesunday irvinesunday

Labels
priority:p1 High priority but not blocking. Causes major but not critical loss of functionality SLA <=7days
Projects
None yet
Milestone
No milestone
2 participants
@andrueastman @irvinesunday