Open AML14 opened 3 years ago
This issue may serve as a starting point for a discussion with the community. We (my team and I) are willing to implement the feature for some programming languages (e.g., Java and Python). But before starting, we would like to get your feedback on it.
For example:
Hello.
I have implemented a first version for this issue that generates Java assertions from IDL, adding support for the x-dependencies
extension
I added the necessary methods for processing IDL in the AbstractJavaCodegen
class, adding the IDL expressions and generated assertions using the vendorExtensions
attribute.
As an example, this is a Youtube API operation in OAS with its dependencies expressed in IDL (in the x-dependencies section):
/youtube/v3/commentThreads:
get:
description: Retrieves a list of resources, possibly filtered.
operationId: youtube.commentThreads.list
parameters:
- description: The *part* parameter specifies a comma-separated list of one or more commentThread resource properties that the API response will include.
explode: true
in: query
name: part
required: true
schema:
items:
type: string
type: array
style: form
- description: Returns the comment threads of all videos of the channel and the channel comments as well.
in: query
name: allThreadsRelatedToChannelId
schema:
type: string
- description: Returns the comment threads for all the channel comments (ie does not include comments left on videos).
in: query
name: channelId
schema:
type: string
- description: Returns the comment threads with the given IDs for Stubby or Apiary.
explode: true
in: query
name: id
schema:
items:
type: string
type: array
style: form
- description: The *maxResults* parameter specifies the maximum number of items that should be returned in the result set.
in: query
name: maxResults
schema:
maximum: 100
minimum: 1
type: integer
- description: "Limits the returned comment threads to those with the specified moderation status. Not compatible with the 'id' filter. Valid values: published, heldForReview, likelySpam."
in: query
name: moderationStatus
schema:
enum:
- published
- heldForReview
- likelySpam
- rejected
type: string
- in: query
name: order
schema:
enum:
- orderUnspecified
- time
- relevance
type: string
- description: The *pageToken* parameter identifies a specific page in the result set that should be returned. In an API response, the nextPageToken and prevPageToken properties identify other pages that could be retrieved.
in: query
name: pageToken
schema:
type: string
- description: Limits the returned comment threads to those matching the specified key words. Not compatible with the 'id' filter.
in: query
name: searchTerms
schema:
type: string
- description: The requested text format for the returned comments.
in: query
name: textFormat
schema:
enum:
- textFormatUnspecified
- html
- plainText
type: string
- description: Returns the comment threads of the specified video.
in: query
name: videoId
schema:
type: string
x-dependencies:
- OnlyOne(allThreadsRelatedToChannelId, channelId, id, videoId);
- ZeroOrOne(id, maxResults);
- ZeroOrOne(id, moderationStatus);
- ZeroOrOne(id, order);
- ZeroOrOne(id, pageToken);
- ZeroOrOne(id, searchTerms);
responses:
"200":
content:
application/json:
schema:
$ref: "#/components/schemas/CommentThreadListResponse"
description: Successful response
"400":
description: 400
"403":
description: 403
"404":
description: 404
security:
- Oauth2:
- https://www.googleapis.com/auth/youtube.force-ssl
Oauth2c:
- https://www.googleapis.com/auth/youtube.force-ssl
tags:
- commentThreads
This OAS generates the following assertions inside the Spring method that handles the request:
// Check dependency: OnlyOne(allThreadsRelatedToChannelId, channelId, id, videoId);
if(!(((!((allThreadsRelatedToChannelId != null))) || ((!((channelId != null))) && (!((id != null && !id.isEmpty()))) && (!((videoId != null))))) && ((!((channelId != null))) || ((!((allThreadsRelatedToChannelId != null))) && (!((id != null && !id.isEmpty()))) && (!((videoId != null))))) && ((!((id != null && !id.isEmpty()))) || ((!((allThreadsRelatedToChannelId != null))) && (!((channelId != null))) && (!((videoId != null))))) && ((!((videoId != null))) || ((!((allThreadsRelatedToChannelId != null))) && (!((channelId != null))) && (!((id != null && !id.isEmpty()))))) && (((allThreadsRelatedToChannelId != null)) || ((channelId != null)) || ((id != null && !id.isEmpty())) || ((videoId != null))))){
return new ResponseEntity<>("Dependency not satisfied: OnlyOne(allThreadsRelatedToChannelId, channelId, id, videoId);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, maxResults);
if(!(((!((id != null && !id.isEmpty()))) || ((!((maxResults != null))))) && ((!((maxResults != null))) || ((!((id != null && !id.isEmpty()))))))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, maxResults);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, moderationStatus);
if(!(((!((id != null && !id.isEmpty()))) || ((!((moderationStatus != null))))) && ((!((moderationStatus != null))) || ((!((id != null && !id.isEmpty()))))))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, moderationStatus);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, order);
if(!(((!((id != null && !id.isEmpty()))) || ((!((order != null))))) && ((!((order != null))) || ((!((id != null && !id.isEmpty()))))))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, order);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, pageToken);
if(!(((!((id != null && !id.isEmpty()))) || ((!((pageToken != null))))) && ((!((pageToken != null))) || ((!((id != null && !id.isEmpty()))))))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, pageToken);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, searchTerms);
if(!(((!((id != null && !id.isEmpty()))) || ((!((searchTerms != null))))) && ((!((searchTerms != null))) || ((!((id != null && !id.isEmpty()))))))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, searchTerms);", HttpStatus.BAD_REQUEST);
}
...
The generated assertions aren't pretty, but they are valid and do their job. I'm currently working on a second version that would reduce this assertions to:
if(!OnlyOneDependency((allThreadsRelatedToChannelId != null),(channelId != null),(id != null && !id.isEmpty()),(videoId != null))){
return new ResponseEntity<>("Dependency not satisfied: OnlyOne(allThreadsRelatedToChannelId, channelId, id, videoId);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, maxResults);
if(!ZeroOrOneDependency((id != null && !id.isEmpty()),(maxResults != null))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, maxResults);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, moderationStatus);
if(!ZeroOrOneDependency((id != null && !id.isEmpty()),(moderationStatus != null))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, moderationStatus);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, order);
if(!ZeroOrOneDependency((id != null && !id.isEmpty()),(order != null))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, order);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, pageToken);
if(!ZeroOrOneDependency((id != null && !id.isEmpty()),(pageToken != null))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, pageToken);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, searchTerms);
if(!ZeroOrOneDependency((id != null && !id.isEmpty()),(searchTerms != null))){
return new ResponseEntity<>("Dependency not satisfied: ZeroOrOne(id, searchTerms);", HttpStatus.BAD_REQUEST);
}
...
The OnlyOneDependency
and ZeroOrOneDependency
methods (and also AllOrNoneDependency
and OrDependency
, not included in this example) would be added to the generated project at static methods in an utility class.
The code for the first version is available in the following repository, which includes the youtube.yaml
file with the previous method and another two example methods.
The generators supported in this demo are the following Java client library:
And the following servers:
To add more libraries and frameworks, you only have to add the assertion section to the appropriate place in the templates, but I don't have experience with most of the libraries, so I did not add them.
The idea would be to add this to the project as an experimental feature for Java and Python, and I would like to receive some feedback from the community about the currently implemented code. Thank you very much!! 😄
Thanks a lot for the work @enriquebarba97, this looks great. If this works well, I would suggest to open a PR following the contributing guidelines. But I also agree that it would be good to get some feedback before, either from the core developers of OpenAPI Generator, or from the community.
Since this feature currently targets specific versions of clients and servers for Java, it may be a good idea to reach out to the main Java developers of the project (listed here).
As Enrique says, future languages and frameworks should be easy to add, by taking Enrique's implementation as a starting point.
I have finished an implementation that can be ready for a pull request. However, I still have some questions about some details on the implementation that I would like to receive feedback on.
To implement the processing of IDL, I added the necessary logic in the AbstractJavaCodegen
class, adding to the fromOperation
method. The specific flow is:
x-dependencies
extension in fromOperation
.CodegenDependency
, so they can be addressed in mustache. CodegenDependency
has a dependency
attribute with the IDL dependency to write in the exception messages, and an assertion
attribute with the assertion in Java language.vendorExtensions
section in the CodegenOperation
object.The section added to the mustache templates looks like the following:
{{#vendorExtensions}}{{#x-dependencies}}
// Check dependency: {{& idlDependency}}
if(!{{& assertOperation}}){
throw new ApiException(400, "Dependency not satisfied: {{& idlDependency}}");
}
{{/x-dependencies}}{{/vendorExtensions}}
The assertions generated are:
IF condition THEN consequence
) as (!(condition) || (consequence))
in Java syntax.e.g. p1<p2
).e.g. p1+p2<100
).This is how the example from the previous comment would look like in a Spring server stub:
// Check dependency: OnlyOne(allThreadsRelatedToChannelId, channelId, id, videoId);
if(!DependencyUtil.OnlyOneDependency((allThreadsRelatedToChannelId != null),(channelId != null),(id != null && !id.isEmpty()),(videoId != null))){
return new ResponseEntity("Dependency not satisfied: OnlyOne(allThreadsRelatedToChannelId, channelId, id, videoId);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, maxResults);
if(!DependencyUtil.ZeroOrOneDependency((id != null && !id.isEmpty()),(maxResults != null))){
return new ResponseEntity("Dependency not satisfied: ZeroOrOne(id, maxResults);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, moderationStatus);
if(!DependencyUtil.ZeroOrOneDependency((id != null && !id.isEmpty()),(moderationStatus != null))){
return new ResponseEntity("Dependency not satisfied: ZeroOrOne(id, moderationStatus);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, order);
if(!DependencyUtil.ZeroOrOneDependency((id != null && !id.isEmpty()),(order != null))){
return new ResponseEntity("Dependency not satisfied: ZeroOrOne(id, order);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, pageToken);
if(!DependencyUtil.ZeroOrOneDependency((id != null && !id.isEmpty()),(pageToken != null))){
return new ResponseEntity("Dependency not satisfied: ZeroOrOne(id, pageToken);", HttpStatus.BAD_REQUEST);
}
// Check dependency: ZeroOrOne(id, searchTerms);
if(!DependencyUtil.ZeroOrOneDependency((id != null && !id.isEmpty()),(searchTerms != null))){
return new ResponseEntity("Dependency not satisfied: ZeroOrOne(id, searchTerms);", HttpStatus.BAD_REQUEST);
}
The feature is mostly finished, and the main code can be checked here, but I ran into some problems and I would like to get some feedback.
To add the static methods for the predefined dependencies, I added a DependenciesUtil.mustache
file with the implementation of the methods. Right now, I generate the Java file by checking if the x-dependencies
extensions are present in the post-processing of the operation, and add the file to the supportingFiles
list (see here). This is implemented just for JavaClientCodegen
for now. However, I believe this is not the most appropriate way of doing this, so I would like some suggestions for this JavaClientCodegen
and other Java generators as well.
Any other feedback about the code is also welcome. If everything is fine, my plan is to do some clean up and open a pull request sometime next week. I will mention some people from the team and the Java technical committee so this does not get lost. Thank you very much for your help! 😄
@wing328 @bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @nmuesch
No news about the integration of this PR ?
I really would like to use this wonderful work in my API.
Is your feature request related to a problem? Please describe.
For a long time, OpenAPI has not supported the specification of inter-parameter dependencies, even though there's an open issue requesting support for it. This issue has become the most upvoted of all times in the OpenAPI repository.
An example of an inter-parameter dependency is "if parameter
p1
is used, thenp2
must be set to'A'
".If these dependencies could be expressed in a machine-readable way, it would be possible to automate lots of things, including the generation of the source code in charge of validating these dependencies in clients and servers.
We have recently proposed an alternative to specify and automatically analyze these dependencies. It is all summed up in the following two Medium posts, for if you have interest:
With our proposal, it is possible to specify inter-parameter dependencies using IDL, a DSL specifically tailored for expressing inter-parameter dependencies in web APIs. What I would like to propose in this feature request is to automatically generate built-in assertions in the source code (client or server, any programming language) based on the dependencies expressed in IDL.
Describe the solution you'd like
We have implemented IDL as an Xtext DSL. Since it's written in Java, and so is OpenAPI generator, I think we could use the IDL parser for generating the assertions in the code, based on the IDL dependencies. For example, if we found a dependency like the following one:
We would write the following assertion in a Java server:
Additional context
The two Medium posts from above summarize pretty much everything there's to know about inter-parameter dependencies. Here's a list of some other resources that you may find interesting: