-
Notifications
You must be signed in to change notification settings - Fork 50
[Dev] Generate CPP code from the OpenAPI REST spec #157
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
Open
Tishj
wants to merge
65
commits into
duckdb:main
Choose a base branch
from
Tishj:generate_response_objects
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…esult objects from the json
…d compilation errors
…<string>', add better ordering to properties
…r that references it
…Type' and 'Expression')
…generated formatting is correct (not super relevant since we have a formatter, but still)
…single entry into that single entry instead
Tishj
commented
Apr 7, 2025
Tishj
commented
Apr 7, 2025
'public:', | ||
f'\tstatic {self.name} FromJSON(yyjson_val *obj);', | ||
'public:', | ||
'\tstring TryFromJSON(yyjson_val *obj);', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought, maybe (as future work) we want to create a stack of errors:
bool TryFromJSON(yyjson_val *obj, stack<string> &error);
Especially since we have these Object[0-9]+
classes, these errors aren't very descriptive if they happen at deeper levels.
With a stack we can use:
if (!TryFromJSON(val, error)) {
error.push("<ClassName> failed to parse");
return false;
}
…'IRCAPI::GetTableCredentials'
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR creates CPP header+source files for all the
schemas
in the Iceberg OpenAPI REST catalog spec.The
yyjson_val*
root is parsed into a tree of PoD/vector/case_insensitive_map_t/<schema class>
, with the exception of:LiteralExpression
'svalue
property.UnaryExpression
'svalue
property.These remain
yyjson_val*
, only because the spec leaves them entirely ambiguous:Parse the OpenAPI spec
First we create a dictionary of
Property
objects from theschemas
in the spec.Whenever we encounter an
object
type inside a property or somewhere that isn't directly part of aschemas
entry, we create a custom ($ref
) entry for it (these are the classes namedObject[0-9]+
).Whenever we encounter an object that consists of only a
allOf
with a single$ref
entry, we deconstruct that object into the entry of theallOf
.(I have been told this is done to be able to include a
description
along with the$ref
in an OpenAPI schema definition)Whenever a property can contain itself (
Expression
,Type
) we mark it as recursive.Generate CPP Code
From a
Property
we create aCPPClass
, this contains all the logic needed to construct the body of theTryFromJSON
method, the variables and the nested classes (theObject[0-9]+
we mentioned earlier).If a property is recursive, it's wrapped in a
unique_ptr
.We generate the following from these
CPPClass
objects:CMakeLists.txt
containing all the generated source files (insrc/rest_catalog/objects
)list.hpp
file containing all the generated header files (insrc/include/rest_catalog/objects
)schemas
(insrc/rest_catalog/objects
)schemas
(insrc/include/rest_catalog/objects
)Future Work
We might benefit from the ability to do the reverse of this (cpp -> JSON) if some of the REST endpoints require json to be sent in the body of the request.
It should be possible to generate the serialization code for this, and perhaps also the helper (setter) methods to populate the object, + the validity method to check compliance with the spec (required fields are populated, allOf schemas are respected, etc..)