Skip to content
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

Improve camelization for words starting with acronym #18218

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -360,4 +360,6 @@ public interface CodegenConfig {

Set<String> getOpenAPIGeneratorIgnoreList();

void setApplyCamelizeFix(boolean applyCamelizeFix);

}
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ apiTemplateFiles are for API outputs only (controllers/handlers).
// Whether to automatically hardcode params that are considered Constants by OpenAPI Spec
protected boolean autosetConstants = false;

// when set to true, apply camelization fix
protected boolean applyCamelizeFix = false;

public boolean getAddSuffixToDuplicateOperationNicknames() {
return addSuffixToDuplicateOperationNicknames;
}
Expand Down Expand Up @@ -6181,13 +6184,30 @@ public String removeNonNameElementToCamelCase(String name) {
* @return camelized string
*/
protected String removeNonNameElementToCamelCase(final String name, final String nonNameElementPattern) {
String result = Arrays.stream(name.split(nonNameElementPattern))
.map(StringUtils::capitalize)
.collect(Collectors.joining(""));
if (result.length() > 0) {
result = result.substring(0, 1).toLowerCase(Locale.ROOT) + result.substring(1);
if (Boolean.parseBoolean(System.getProperty("openapi.generator.fix.camelize"))) {
// new behaviour with fix
String[] splitString = name.split(nonNameElementPattern);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change it to this so that it does not mutate the string in unintended ways? We just want to change casing, not strip whitespace.

String[] splitString = name.split(nonNameElementPattern, -1);


if (splitString.length > 0) {
splitString[0] = camelize(splitString[0], CamelizeOption.LOWERCASE_FIRST_CHAR);
}

String result = Arrays.stream(splitString)
.map(StringUtils::capitalize)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we capitalize splitString[0] when we just used camelize with lower case first?

.collect(Collectors.joining(""));
if (result.length() > 0) {
result = result.substring(0, 1).toLowerCase(Locale.ROOT) + result.substring(1);
}
return result;
} else { // old behaviour with bug
String result = Arrays.stream(name.split(nonNameElementPattern))
.map(StringUtils::capitalize)
.collect(Collectors.joining(""));
if (result.length() > 0) {
result = result.substring(0, 1).toLowerCase(Locale.ROOT) + result.substring(1);
}
return result;
}
return result;
}

@Override
Expand Down Expand Up @@ -8502,6 +8522,8 @@ public void setAutosetConstants(boolean autosetConstants) {
this.autosetConstants = autosetConstants;
}

public void setApplyCamelizeFix(boolean applyCamelizeFix) { this.applyCamelizeFix = applyCamelizeFix; }

/**
* This method removes all constant Query, Header and Cookie Params from allParams and sets them as constantParams in the CodegenOperation.
* The definition of constant is single valued required enum params.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.openapitools.codegen;

import com.fasterxml.jackson.databind.annotation.JsonAppend;
import io.swagger.v3.core.util.Json;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.Operation;
Expand Down Expand Up @@ -257,6 +258,13 @@ void configureGeneratorProperties() {
System.out.println(SerializerUtils.toJsonString(openAPI));
}

// check to see if we need to apply camelize fix
if (config.additionalProperties().containsKey("applyCamelizeFix")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we think the new method is better, then we should make it enabled by default with an option to opt out and use the legacy method.

Copy link
Member Author

Choose a reason for hiding this comment

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

i want to let users to try it out first to see if we've other edge cases not yet handled.

plan to flip the switch in the upcoming v8.0.0 release.

org.openapitools.codegen.utils.StringUtils.applyCamelizeFix =
Boolean.parseBoolean(String.valueOf(config.additionalProperties().get("applyCamelizeFix")));
config.setApplyCamelizeFix(true);
}

config.processOpts();
if (opts != null && opts.getGeneratorSettings() != null) {
config.typeMapping().putAll(opts.getGeneratorSettings().getTypeMappings());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ public class StringUtils {
*/
public static final String NAME_CACHE_EXPIRY_PROPERTY = "org.openapitools.codegen.utils.namecache.expireafter.seconds";

// if set true, enable the camelize fix
public static boolean applyCamelizeFix = false;

// A cache of camelized words. The camelize() method is invoked many times with the same
// arguments, this cache is used to optimized performance.
private static Cache<Pair<String, CamelizeOption>, String> camelizedWordsCache;
Expand All @@ -39,6 +42,8 @@ public class StringUtils {
// A cache of escaped words, used to optimize the performance of the escape() method.
private static Cache<EscapedNameOptions, String> escapedWordsCache;



static {
int cacheSize = Integer.parseInt(GlobalSettings.getProperty(NAME_CACHE_SIZE_PROPERTY, "200"));
int cacheExpiry = Integer.parseInt(GlobalSettings.getProperty(NAME_CACHE_EXPIRY_PROPERTY, "5"));
Expand All @@ -59,6 +64,7 @@ public class StringUtils {
.expireAfterAccess(cacheExpiry, TimeUnit.SECONDS)
.ticker(Ticker.systemTicker())
.build();

}

private static Pattern capitalLetterPattern = Pattern.compile("([A-Z]+)([A-Z][a-z][a-z]+)");
Expand Down Expand Up @@ -117,6 +123,7 @@ public static String camelize(String word) {

private static Pattern camelizeSlashPattern = Pattern.compile("\\/(.?)");
private static Pattern camelizeUppercasePattern = Pattern.compile("(\\.?)(\\w)([^\\.]*)$");
private static Pattern camelizeUppercaseStartPattern = Pattern.compile("^([A-Z]+)(([A-Z][a-z].*)|([^a-zA-Z].*)|$)$");
private static Pattern camelizeUnderscorePattern = Pattern.compile("(_)(.)");
private static Pattern camelizeHyphenPattern = Pattern.compile("(-)(.)");
private static Pattern camelizeDollarPattern = Pattern.compile("\\$");
Expand All @@ -135,8 +142,18 @@ public static String camelize(final String inputWord, CamelizeOption camelizeOpt
return camelizedWordsCache.get(key, pair -> {
String word = pair.getKey();
CamelizeOption option = pair.getValue();

Matcher m;
// Lowercase acronyms at start of word if not UPPERCASE_FIRST_CHAR
if (applyCamelizeFix) {
m = camelizeUppercaseStartPattern.matcher(word);
if (camelizeOption != UPPERCASE_FIRST_CHAR && m.find()) {
word = m.group(1).toLowerCase(Locale.ROOT) + m.group(2);
}
}

// Replace all slashes with dots (package separator)
Matcher m = camelizeSlashPattern.matcher(word);
m = camelizeSlashPattern.matcher(word);
while (m.find()) {
word = m.replaceFirst("." + m.group(1).replace("\\", "\\\\")/*.toUpperCase()*/);
m = camelizeSlashPattern.matcher(word);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4913,4 +4913,16 @@ public void testWebhooks() throws IOException {
Assert.assertEquals(co.operationId, "newPetGet");
}

public void testRemoveNonNameElementToCamelCase() {
final DefaultCodegen codegen = new DefaultCodegen();

final String alreadyCamelCase = "aVATRate";
Assert.assertEquals(codegen.removeNonNameElementToCamelCase(alreadyCamelCase), alreadyCamelCase);

final String startWithCapitals = "VATRate";
Assert.assertEquals(codegen.removeNonNameElementToCamelCase(startWithCapitals), "vatRate");

final String startWithCapitalsThenNonNameElement = "DELETE_Invoice";
Assert.assertEquals(codegen.removeNonNameElementToCamelCase(startWithCapitalsThenNonNameElement), "deleteInvoice");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ public Object[][] provideTestData()
{ "foo-bar", "fooBar", camelCase },
{ "foo_bar", "fooBar", camelCase },
{ "foo bar", "fooBar", camelCase },
{ "FOO-BAR", "fOOBAR", camelCase }, // camelize doesn't support uppercase
{ "FOO_BAR", "fOOBAR", camelCase }, // ditto
// { "FOO-BAR", "fOOBAR", camelCase }, // camelize doesn't support uppercase
// { "FOO_BAR", "fOOBAR", camelCase }, // ditto
{ "FooBar", "FooBar", PascalCase },
{ "fooBar", "FooBar", PascalCase },
{ "foo-bar", "FooBar", PascalCase },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ public void testEnumPropertyWithDefaultValue() {
CodegenProperty cp1 = cm1.vars.get(0);
Assert.assertEquals(cp1.getEnumName(), "PropertyName");
Assert.assertEquals(cp1.getDefaultValue(), "PropertyName.VALUE");
//Assert.assertEquals(cp1.getDefaultValue(), "PropertyName.`value`");
}

@Test(description = "Issue #3804")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ public void testCamelize() throws Exception {

Assert.assertEquals(camelize("some-value", LOWERCASE_FIRST_CHAR), "someValue");
Assert.assertEquals(camelize("$type", LOWERCASE_FIRST_CHAR), "$Type");

Assert.assertEquals(camelize("aVATRate", LOWERCASE_FIRST_CHAR), "aVATRate");
//Assert.assertEquals(camelize("VATRate", LOWERCASE_FIRST_CHAR), "vatRate");
//Assert.assertEquals(camelize("DELETE_Invoice", LOWERCASE_FIRST_CHAR), "deleteInvoice");

//Assert.assertEquals(camelize("aVATRate"), "AVATRate");
//Assert.assertEquals(camelize("VATRate"), "VATRate");
//Assert.assertEquals(camelize("DELETE_Invoice"), "DELETEInvoice");
}

@Test
Expand Down
Loading
Loading