[Apex] Set namedCredential file name from the property #2937
Conversation
loganm
force-pushed
the
apex-fix-named-credential-property
branch
from
6d2d655 to
2f92af9
Compare
| Info info = openAPI.getInfo(); | ||
| String calloutLabel = info.getTitle(); | ||
| String calloutLabel = openAPI.getInfo().getTitle(); | ||
| if (namedCredential != null && !namedCredential.trim().isEmpty()) { |
There was a problem hiding this comment.
What about using StringUtils.isNotEmpty instead?
There was a problem hiding this comment.
@wing328 Very good idea. I'm not a Java developer by trade, so I'm a bit gun shy with libraries.
That looks much better.
loganm
force-pushed
the
apex-fix-named-credential-property
branch
from
2f92af9 to
f73852b
Compare
loganm
changed the title
loganm
force-pushed
the
apex-fix-named-credential-property
branch
from
f73852b to
177cef6
Compare
The namedCredential property is intended to override the namedCredentials name, but it wasn't being used. Instead, the title from the openapi spec file was used, and the namedCredential property wasn't used anywhere. For backwards compatibility, this change also keeps the spec title as the default name if an override is not provided.
loganm
force-pushed
the
apex-fix-named-credential-property
branch
from
177cef6 to
0ccee13
Compare
| @@ -46,7 +47,7 @@ | |||
| private String classPrefix = "OAS"; | |||
| private String apiVersion = "42.0"; | |||
| private String buildMethod = "sfdx"; | |||
| private String namedCredential = classPrefix; | |||
| private String namedCredential; | |||
There was a problem hiding this comment.
@loganm What about keeping the default as classPrefix?
There was a problem hiding this comment.
The current behaviour of the generator is to use the spec title for the namedCredential. For example, I'm using a Dun and Bradstreet spec with the title 'Search_for_a_Company_or_Contact'.
The default classPrefix is 'OAS' which doesn't mean much. The spec title is more informative about purpose of the named credential.
Now that the namedCredential var is being referenced, leaving the default value as classPrefix here would also change the default behaviour of the generator. It could be a breaking change for anyone using it presently.
Do you think it's possible to get at the spec title as the default here instead of the class prefix?
There was a problem hiding this comment.
Now that the namedCredential var is being referenced, leaving the default value as classPrefix here would also change the default behavior of the generator. It could be a breaking change for anyone using it presently.
Yes, we're on the same page to make this change backward-compatible.
Let me review again as I thought the default value is no longer the same.
There was a problem hiding this comment.
I just updated the petstore samples and I can see the issue you've fixed in this PR 👍
|
I've updated the Apex samples and add apex-petstore.sh to ensure-up-to-date script via 5ad318b |
|
@loganm thanks again for your contribution, which has been included in the v4.0.1 release (https://twitter.com/oas_generator/status/1135534738658062336) |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh,./bin/openapi3/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.master,4.1.x,5.0.x. Default:master.Description of the PR
@asnelling It seems like you're the person to review this one.
This fixes #2904
The namedCredential property is intended to override the namedCredentials name, but it wasn't being used. Instead, the title from the openapi spec file was used in all cases, and the namedCredential property wasn't used anywhere.
If a namedCredential property is provided, it will be used instead of the default name. For backwards compatibility, the spec title is still used for the named credential by default.