Skip to content

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.

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

Merged
merged 14 commits into from
Oct 6, 2025
Merged

CLOUDP-345026: Update the L1 generation tool to support x-xgen-operation-id-override #4233

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2f44fe9
Update L1 generation to use x-gen-operation-id-override
Luke-Sanderson Sep 24, 2025
4910323
Update test snapshots
Luke-Sanderson Sep 24, 2025
0d0fe79
Ran make gen-api-commands
Luke-Sanderson Sep 24, 2025
3f4dd64
Re-add no-lint comments
Luke-Sanderson Sep 24, 2025
ba96893
Does not add operationId to alias when overriden by x-xgen-atlascli
Luke-Sanderson Sep 25, 2025
f28c669
Add operationid override snapshot test
Luke-Sanderson Sep 25, 2025
b0c032b
Update contributing.md
Luke-Sanderson Sep 25, 2025
d85b0fe
Use := instead of var
Luke-Sanderson Sep 25, 2025
eaa9ab9
Move alias logic out of ExtractExtensionsFromOperation
Luke-Sanderson Sep 26, 2025
84ac910
Remove duplicate check
Luke-Sanderson Sep 26, 2025
cacaf24
Remove duplicate alias append and set cobra command operationID
Luke-Sanderson Sep 26, 2025
22859db
Update snapshot
Luke-Sanderson Sep 26, 2025
3ba0e64
Revert removal of operationId override
Luke-Sanderson Oct 6, 2025
cf73247
Add comment about multiple operationID overrides for clarity
Luke-Sanderson Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions internal/cli/api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,11 @@ func convertAPIToCobraCommand(command shared_api.Command) (*cobra.Command, error
commandName := strcase.ToLowerCamel(command.OperationID)
commandAliases := command.Aliases

// Prefer to use shortOperationID if present
if command.ShortOperationID != "" {
// Add original operation ID to aliases
commandAliases = append(commandAliases, commandName)
// Use shortOperationID as command name
commandName = strcase.ToLowerCamel(command.ShortOperationID)
commandAliases = append(commandAliases, strcase.ToLowerCamel(command.OperationID))
}

shortDescription, longDescription := splitShortAndLongDescription(command.Description)
Expand Down
64 changes: 19 additions & 45 deletions tools/cmd/api-generator/convert_commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,55 +113,32 @@ func extractExtensionsFromOperation(operation *openapi3.Operation) operationExte
operationAliases: []string{},
}

processAtlasCLIExtensions(&ext, operation.Extensions)
processShortOperationIDOverride(&ext, operation)

return ext
}

func processAtlasCLIExtensions(ext *operationExtensions, extensions map[string]any) {
atlasCLIExt, ok := extensions["x-xgen-atlascli"].(map[string]any)
if !ok || atlasCLIExt == nil {
return
if shortOperationID, ok := operation.Extensions["x-xgen-operation-id-override"].(string); ok && shortOperationID != "" {
ext.shortOperationID = shortOperationID
}

if extSkip, okSkip := atlasCLIExt["skip"].(bool); okSkip && extSkip {
ext.skip = extSkip
}
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
}

if extAliases, okExtAliases := atlasCLIExt["command-aliases"].([]any); okExtAliases && extAliases != nil {
for _, alias := range extAliases {
if sAlias, ok := alias.(string); ok && sAlias != "" {
ext.operationAliases = append(ext.operationAliases, sAlias)
if extAliases, okExtAliases := extensions["command-aliases"].([]any); okExtAliases && extAliases != nil {
for _, alias := range extAliases {
if sAlias, ok := alias.(string); ok && sAlias != "" {
ext.operationAliases = append(ext.operationAliases, sAlias)
}
}
}
}

if overrides := extractOverrides(extensions); overrides != nil {
if overriddenOperationID, ok := overrides["operationId"].(string); ok && overriddenOperationID != "" {
ext.operationID = overriddenOperationID
if overrides := extractOverrides(operation.Extensions); overrides != nil {
if overriddenOperationID, ok := overrides["operationId"].(string); ok && overriddenOperationID != "" {
ext.operationID = overriddenOperationID
ext.shortOperationID = ""
}
}
}
}

func processShortOperationIDOverride(ext *operationExtensions, operation *openapi3.Operation) {
shortOperationID, ok := operation.Extensions["x-xgen-operation-id-override"].(string)
if !ok || shortOperationID == "" {
return
}

ext.shortOperationID = shortOperationID

// Only add original operation ID to aliases if there's no Atlas CLI operation ID override
overrides := extractOverrides(operation.Extensions)
if overrides == nil {
ext.operationAliases = append(ext.operationAliases, strcase.ToLowerCamel(operation.OperationID))
return
}

if _, hasOverride := overrides["operationId"].(string); !hasOverride {
ext.operationAliases = append(ext.operationAliases, strcase.ToLowerCamel(operation.OperationID))
}
return ext
}

func operationToCommand(now time.Time, path, verb string, operation *openapi3.Operation) (*api.Command, error) {
Expand Down Expand Up @@ -198,11 +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 {
Copy link
Collaborator

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 of x-xgen-operation-id-override? I'm trying to understand if we haven't missed any overrides here that should've been translated to x-xgen-operation-id-override

Copy link
Collaborator Author

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 by x-xgen-operation-id-override in the TD so I think I removed this my mistake. Will add it back in

Copy link
Collaborator

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

Copy link
Collaborator Author

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 as x-operationid right?

Copy link
Collaborator

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?

Copy link
Collaborator Author

@Luke-Sanderson Luke-Sanderson Oct 6, 2025 •
edited
Loading

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

if overriddenOperationID, ok := overrides["operationId"].(string); ok && overriddenOperationID != "" {
operationID = overriddenOperationID
shortOperationID = ""
}
if shortOperationID != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this logic? Since we already to the same thing in cli/api?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not and I have now removed it

aliases = append(aliases, strcase.ToLowerCamel(operationID))
}

watcher, err := extractWatcherProperties(operation.Extensions)
Expand Down
Loading