-
Notifications
You must be signed in to change notification settings - Fork 89
CLOUDP-345026: Update the L1 generation tool to support x-xgen-operation-id-override #4233
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
Changes from 10 commits
2f44fe9
4910323
0d0fe79
3f4dd64
ba96893
f28c669
b0c032b
d85b0fe
eaa9ab9
84ac910
cacaf24
22859db
3ba0e64
cf73247
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,16 +101,22 @@ func extractSunsetDate(extensions map[string]any) *time.Time { | |
type operationExtensions struct { | ||
skip bool | ||
operationID string | ||
shortOperationID string | ||
operationAliases []string | ||
} | ||
|
||
func extractExtensionsFromOperation(operation *openapi3.Operation) operationExtensions { | ||
ext := operationExtensions{ | ||
skip: false, | ||
operationID: operation.OperationID, | ||
shortOperationID: "", | ||
operationAliases: []string{}, | ||
} | ||
|
||
if shortOperationID, ok := operation.Extensions["x-xgen-operation-id-override"].(string); ok && shortOperationID != "" { | ||
ext.shortOperationID = shortOperationID | ||
} | ||
|
||
if extensions, okExtensions := operation.Extensions["x-xgen-atlascli"].(map[string]any); okExtensions && extensions != nil { | ||
if extSkip, okSkip := extensions["skip"].(bool); okSkip && extSkip { | ||
ext.skip = extSkip | ||
|
@@ -127,6 +133,7 @@ func extractExtensionsFromOperation(operation *openapi3.Operation) operationExte | |
if overrides := extractOverrides(operation.Extensions); overrides != nil { | ||
if overriddenOperationID, ok := overrides["operationId"].(string); ok && overriddenOperationID != "" { | ||
ext.operationID = overriddenOperationID | ||
ext.shortOperationID = "" | ||
} | ||
} | ||
} | ||
|
@@ -141,6 +148,7 @@ func operationToCommand(now time.Time, path, verb string, operation *openapi3.Op | |
} | ||
|
||
operationID := extensions.operationID | ||
shortOperationID := extensions.shortOperationID | ||
aliases := extensions.operationAliases | ||
|
||
httpVerb, err := api.ToHTTPVerb(verb) | ||
|
@@ -167,10 +175,8 @@ func operationToCommand(now time.Time, path, verb string, operation *openapi3.Op | |
return nil, fmt.Errorf("failed to clean description: %w", err) | ||
} | ||
|
||
if overrides := extractOverrides(operation.Extensions); overrides != nil { | ||
if overriddenOperationID, ok := overrides["operationId"].(string); ok && overriddenOperationID != "" { | ||
operationID = overriddenOperationID | ||
} | ||
if shortOperationID != "" { | ||
|
||
aliases = append(aliases, strcase.ToLowerCamel(operationID)) | ||
} | ||
|
||
watcher, err := extractWatcherProperties(operation.Extensions) | ||
|
@@ -179,9 +185,10 @@ func operationToCommand(now time.Time, path, verb string, operation *openapi3.Op | |
} | ||
|
||
command := api.Command{ | ||
OperationID: operationID, | ||
Aliases: aliases, | ||
Description: description, | ||
OperationID: operationID, | ||
ShortOperationID: shortOperationID, | ||
Aliases: aliases, | ||
Description: description, | ||
RequestParameters: api.RequestParameters{ | ||
URL: path, | ||
QueryParameters: parameters.query, | ||
|
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.
[q] for my own understanding, we have removed
operationId
overrides officially in benefit ofx-xgen-operation-id-override
? I'm trying to understand if we haven't missed any overrides here that should've been translated tox-xgen-operation-id-override
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.
There is no mention of replacing the
operationId
override officially byx-xgen-operation-id-override
in the TD so I think I removed this my mistake. Will add it back inThere 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.
but does it make sense to keep two types of overrides? I think my main concern is just making sure we're handling both if they exist, but we could look into how the overrides are being added at the moment and if we should rename them, or maybe, we don't even use them
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.
Since both target the same field (operationID) one will override the other. From what I can see in the latest openapi spec, I don't think
operationID
override is used. It was appear asx-operationid
right?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.
interesting! I think this is the AtlasCLI framework for overrides, e.g. search for the openapi spec tests with
x-xgen-atlascli
. There is a different format there for different overrides. Perharps by keeping this here, the idea is that if we need to override via AtlasCLI we can, let's drop a comment here maybe capturing this behaviour in case we see this in the future?Uh oh!
There was an error while loading. Please reload this page.
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.
Oh yes this is the
x-xgen-atlascli
one. I think I might have originally removed it because I thought it was duplicating the logic from here. Yes I agree there is a use case for this override for overriding the operationId only for atlascli and I think that this override should take priority.I will add a comment to reflect this