search

camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.1k stars 1.55k forks source link

OpenAPI: Set non-optional properties as required in responses #2548

Open ThorbenLindhauer opened 3 years ago

ThorbenLindhauer commented 3 years ago

This issue was imported from JIRA:

Field Value
JIRA Link CAM-12815
Reporter D6pN5Pr
What is this name? This pseudonym name was generated based on the user name in JIRA to protect the personal data of our JIRA users. You can use this identifier to search for issues by the same reporter.
Has restricted visibility comments true

For nullable aware programming languages like Kotlin it's nice if the OpenAPI spec is explicit about properties that are never null so null safe code can be generated. It looks like required is already being documented for request parameters and bodies, but not for response bodies.

An example is https://docs.camunda.org/manual/latest/reference/rest/process-definition/post-start-process-instance. I don't think "id" in the response is ever supposed to be missing. If so, it would be nice if ProcessInstanceWithVariablesDto has "id" in the required list.

Links:

ThorbenLindhauer commented 3 years ago

This comment was imported from JIRA and written by user @tmetzke

Hi D6pN5Pr,

thanks for this suggestion, this sounds like a very good addition to our OpenAPI specification.

I will forward this to decision-making. You can follow the ticket to stay up-to-date with our progress on it.

In the meantime, if you feel up for it, I'd encourage you to add a Pull Request to our repository to add this feature. This would speed up the addition of this feature tremendously. If you'd like any pointers on how to start, let us know and we'll guide you!

Thanks and best, Tobias

ThorbenLindhauer commented 3 years ago

This comment was imported from JIRA and written by user D6pN5Pr

What is this name? This pseudonym name was generated based on the user name in JIRA to protect the personal data of our JIRA users. You can use this identifier to search for issues by the same reporter.

Hi @Tobias Metzke-Bernstein,

If this suggestion is indeed acceptable I could give a shot at updating (part of) the specification. I'm pretty new to Camunda however so if you have any pointers on how I can verify which properties are never null that would help immensely. The documentation seems to sometimes point out nullability, but not always.

For example https://docs.camunda.org/manual/latest/reference/rest/task/get-query/:

Any other pointers are of course welcome as well.

Kind regards, Davey

ThorbenLindhauer commented 3 years ago

This comment was imported from JIRA and written by user @yanavasileva

Hi D6pN5Pr,

Thank you once again for your interest in improving our OpenAPI documentation. {quote}how I can verify which properties are never null that would help immensely. The documentation seems to sometimes point out nullability, but not always. {quote} Unfortunately, there are no rules or pointers that indicates if a property is a required and not. I hope that won't discourage you of raising a pull request on the topic. You can ask us for specific cases and we will provide you input, however covering the whole Rest API might take a while.

I would like bring you to your attention one more aspect to the nullable properties topic. In some other languages (users creating C# clients raised an issue), for example String or Boolean properties are not nullable by default. There the nullable field needs to be provided for those otherwise the endpoints will have failures with required fields that are not actually required. After looking at that scenario (CAM-12326 ) we made most of fields not nullable so that the endpoints are less restrictive and the C# users could work use the OpenAPI documentation. One of the reasons to lower the strict definition is that DTOs are used as request and response bodies in some endpoints directly or by inheritance, Meaning the fields can be required in some endpoints and mandatory in other but it still belongs to the same DTO (or model/schema in the OpenAPI).

The response body of https://docs.camunda.org/manual/latest/reference/rest/process-definition/post-start-process-instance/ is ProcessInstanceDto. The dto is also part of the MessageCorrelationResultDto. And in this scenario, it will be fine to mark the id property as nullable=false. Unfortunately, I didn't find an example where in one endpoint the property is required and in other not. However, I am certain those exist as already have seen a couple when I worked on previous tickets.

Regarding you question about https://docs.camunda.org/manual/latest/reference/rest/task/get-query/ - a task can belong to a process instance, case instance or it could be a standalone, so process and case properties can be null.

I hope the above information gave you enough insights about the our Rest API and Open API documentation. Please let me know if you have open questions or concerns. Kindly keep in mind that the OpenAPI topic is ongoing for us, we are happy to receive feedback and see that users are interest in using it.

Please let us know if we can support you further when you start working on the pull request.

Best regards, Yana