Skip to content

Don't override businessInformation #74

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

Open
ckuijjer wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Don't override businessInformation #74

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
437ee71
Use OpenAPI contact information only if there's no businessInformatio…
ckuijjer May 24, 2025
3d77ebf
added prettierrc based on the current formatting of the ts files
ckuijjer May 24, 2025
32b4a59
remove .prettierrc, as ESLint can be used instead
ckuijjer May 26, 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
4 changes: 4 additions & 0 deletions .prettierrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"printWidth": 100,
"singleQuote": true
}
21 changes: 12 additions & 9 deletions lib/src/wso2/wso2-api/api-defs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,20 @@ const applyOpenapiAsDefaults = (
apiDef: Wso2ApiDefinitionV1,
): Wso2ApiDefinitionV1 => {
const apiDefr = { ...apiDef };
// user openapi contact info for business/technical information of the api
// use openapi contact info for business/technical information of the api
if (
(!apiDefr.businessInformation && openapiDocument.info.contact?.email) ||
openapiDocument.info.contact?.name
!apiDefr.businessInformation &&
Copy link
Owner

@flaviostutz flaviostutz May 25, 2025

Choose a reason for hiding this comment

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

Even if businessInformation is set, we should use the info from info.contact.email/name, don't we?

Maybe we could change the logic to always try to set the info based on "info.contact" if it exists.

What do you think?

Copy link
Contributor Author

@ckuijjer ckuijjer May 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That's indeed the general design question we need to find a good answer to! I think there's three options.

Let's take the version field as an example and say the OpenAPI document contains a version with value v1, and the user passes a version with value v2, e.g. new Wso2Api(..., ..., { apiDefinition: { version: 'v2' }, openapiDocument })

  1. The values from the OpenAPI document are only used to set defaults in apiDefinition. The user can override this by setting the value in apiDefinition. For the example above, the API gets created as version v2.
  2. The values from OpenAPI document take precedence over values provided by the user. For the example above, the API gets created as version v1.
  3. A mix of 1 and 2, depending on the field. This is the current behavior on main where description, version, and tags use the OpenAPI as a default, and where the four businessInformation fields always come from the OpenAPI document.

I think option 1 is the best as it gives clear defaults, and allows for the user to override these fields where needed.

Let's take a user that wants to learn about publishing to WSO2 and who takes the Petstore OpenAPI document as an example OpenAPI document. Do we want them to make changes to the OpenAPI document and set its contact information correctly, or do we want them to simply pass the correct contact information when writing the new Wso2API code?

Copy link
Owner

@flaviostutz flaviostutz Jun 4, 2025

Choose a reason for hiding this comment

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

Great summary on this matter! I vote for option 2. As if we go for option 1, then we should change the name of the property to "default info xxx" to make it more clear and we don't want a breaking change now :)

The idea of this property was to be a "sugar" attribute to set these info correctly in OpenAI doc, but I see the mixed behavior causes confusion. So let's go to option 1.

What do you think?

(openapiDocument.info.contact?.email || openapiDocument.info.contact?.name)
) {
apiDefr.businessInformation = {
businessOwnerEmail: openapiDocument.info.contact?.email,
technicalOwnerEmail: openapiDocument.info.contact?.email,
technicalOwner: openapiDocument.info.contact?.name,
businessOwner: openapiDocument.info.contact?.name,
};
apiDefr.businessInformation = {};
if (openapiDocument.info.contact?.email) {
apiDefr.businessInformation.businessOwnerEmail = openapiDocument.info.contact?.email;
apiDefr.businessInformation.technicalOwnerEmail = openapiDocument.info.contact?.email;
}
if (openapiDocument.info.contact?.name) {
apiDefr.businessInformation.businessOwner = openapiDocument.info.contact?.name;
apiDefr.businessInformation.technicalOwner = openapiDocument.info.contact?.name;
}
}

if (!apiDefr.description && openapiDocument.info.description) {
Expand Down