Skip to content

Conversation

sphterry
Copy link
Contributor

@sphterry sphterry commented Jul 14, 2025

Added extension property x-xgen-method-verb-override so that legacy custom methods and endpoints with outlying design patterns can have their OperationIDs validated. This will prevent us from having to use exceptions, which makes downstream parsing of OperationIDs more difficult.

The extension includes a verb property that is used in place of the standard method verb and a customMethod boolean that indicates if a method should be validated as part of IPA 109.

This extension will be added to the endpoints listed as anticipated corrections in MMS as part of CLOUDP-331138.

Tests for the extension logic will be added when OperationID Validation rules and tests are enabled (CLOUDP-329722)

Jira ticket: CLOUDP-306294

@sphterry sphterry requested a review from a team as a code owner July 14, 2025 15:43
Comment on lines 11 to 15
for (let i = 0; i < keys.length; i++) {
if (hasVerbOverride(endpoint[keys[i]])) {
return true;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for (let i = 0; i < keys.length; i++) {
if (hasVerbOverride(endpoint[keys[i]])) {
return true;
}
}
return keys.contains(VERB_OVERRIDE_EXTENSION)

Could this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oo yes should do! Will add now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This actually doesn't work because I'm trying to look for extensions in the children of the object. I should be able to make it cleaner with includes though

Comment on lines 18 to 20
let expectedOperationID = '';
if (isCustomMethodIdentifier(resourcePath)) {
expectedOperationID = generateOperationID(getCustomMethodName(resourcePath), stripCustomMethodName(resourcePath));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let expectedOperationID = '';
if (isCustomMethodIdentifier(resourcePath)) {
expectedOperationID = generateOperationID(getCustomMethodName(resourcePath), stripCustomMethodName(resourcePath));
if (isCustomMethodIdentifier(resourcePath)) {
const expectedOperationID = generateOperationID(getCustomMethodName(resourcePath), stripCustomMethodName(resourcePath));

Since you are only using expectedOperationID in the scope of each if block, you can declare it within. the block itself and assign

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same with errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a refactor, thank you!

Comment on lines 48 to 50
for (let i = 0; i < methods.length; i++) {
let obj = input[methods[i]];
const operationId = obj.operationId;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for (let i = 0; i < methods.length; i++) {
let obj = input[methods[i]];
const operationId = obj.operationId;
methods.forEach((method) => {
const operationId = method.operationId
...
})

Copy link
Collaborator

@lovisaberggren lovisaberggren Jul 14, 2025

Choose a reason for hiding this comment

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

Tip: JS has a lot of fun functions on Arrays, so that you rarely need to use for loops: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array 🚀

@sphterry sphterry enabled auto-merge (squash) July 15, 2025 09:37
@sphterry sphterry requested a review from lovisaberggren July 15, 2025 09:38
Copy link
Collaborator

@lovisaberggren lovisaberggren left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments, LGTM! 🚀

@sphterry sphterry merged commit 9a2a9aa into main Jul 15, 2025
8 checks passed
@sphterry sphterry deleted the CLOUDP-306294 branch July 15, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants