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

Fix isenum for referenced enums #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3168,6 +3168,7 @@ public CodegenProperty fromProperty(String name, Schema p) {
if (referencedSchema.getEnum() != null && !referencedSchema.getEnum().isEmpty()) {
List<Object> _enum = referencedSchema.getEnum();

property.isEnum = true;
Map<String, Object> allowableValues = new HashMap<String, Object>();
allowableValues.put("values", _enum);
if (allowableValues.size() > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1339,18 +1339,19 @@ public String toEnumName(CodegenProperty var) {
return toEnumName(var.items);
}
String paramNameType = "E'" + toTypeName("", var.name);
String enumValues = var._enum.toString();
if (var._enum != null) {
String enumValues = var._enum.toString();

Pair<Boolean, String> duplicateEnum = isDuplicateEnumValues(enumValues);
if (duplicateEnum.getLeft()) {
paramNameType = duplicateEnum.getRight();
} else {
paramNameType = toDedupedModelName(paramNameType, enumValues, false);
var.datatypeWithEnum = paramNameType;
updateCodegenPropertyEnum(var);
addEnumToUniques(paramNameType, var.dataType, enumValues, var.allowableValues, var.description);
Pair<Boolean, String> duplicateEnum = isDuplicateEnumValues(enumValues);
if (duplicateEnum.getLeft()) {
paramNameType = duplicateEnum.getRight();
} else {
paramNameType = toDedupedModelName(paramNameType, enumValues, false);
var.datatypeWithEnum = paramNameType;
updateCodegenPropertyEnum(var);
addEnumToUniques(paramNameType, var.dataType, enumValues, var.allowableValues, var.description);
}
}

return paramNameType;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,14 @@ public void postProcessModelProperty(CodegenModel model, CodegenProperty propert
if (!Boolean.TRUE.equals(model.isEnum)) {
model.imports.add("JsonProperty");
if (Boolean.TRUE.equals(model.hasEnums)) {
model.imports.add("JsonValue");
//Loop through properties to check if one is an embedded enum
for (CodegenProperty prop : model.allVars) {
//If a property is an enum and a primitive type, it is an embedded enum
if (prop.isEnum && prop.isPrimitiveType) {
model.imports.add("JsonValue");

Choose a reason for hiding this comment

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

This will have already occurred on line 519.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Not what I witnessed: if the model itself is not an enum, but has variables that are enums, then kotlin generator need to have that import.

... Now that I reread, I see it's hasEnums so you should be right. Two possibilities: my code here changed nothing so I'll remove it, or hasEnumwas false and I need to check out why. I'll try to do that on monday and report back

Copy link
Owner Author

Choose a reason for hiding this comment

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

@jimschubert Just took a look at it again, line 519 adds JsonProperty if a model is not an enum, because it will have properties.
My code replaces the previous line 521 that was adding JsonValue whenever a model has enums by a code that adds it only if one of the enum is embedded, as what was done previously by that generator. I did it to have the same result as master, although the JsonValue import doesn't seem to be used at all...

break;
}
}
}
} else {
//Needed imports for Jackson's JsonCreator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
{{>interfaceOptVar}}
{{/optionalVars}}
{{/discriminator}}
{{#hasEnums}}{{#vars}}{{#isEnum}}
{{#hasEnums}}{{#vars}}{{#isEnum}}{{#isPrimitiveType}}

Choose a reason for hiding this comment

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

Interesting. I started looking at this a bit after our Slack conversation, but hadn't thought about how isPrimitiveType might fit into things.

One issue with considering enums as primitive is that $ref doesn't mean "This is a class/model", it means "I want to reuse this bit of schema in more than one place". So, if an enum which is inline is a primitive type, an enum that is $ref should also be a primitive type.

I'll have to think about this a little more and maybe discuss with the core team.

Copy link
Owner Author

@LukeMarlin LukeMarlin Jul 4, 2020

Choose a reason for hiding this comment

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

Yes, the kotlin test allowed me to notice that an inline enum is basically this in core:

  • isEnum = True
  • isPrimitive = True
  • isString = True

But when you have a referenced enum (with my change), you get:

  • isEnum = True

and that's it.

So relying on isPrimitive would be the only way for generators to distinguish between the two cases (as far as I've noticed).
It does seem a bit brittle though. I guess isString might be ok, but I'm unsure it would cover all cases.

/**
* {{{description}}}
* Values: {{#allowableValues}}{{#enumVars}}{{&name}}{{^-last}},{{/-last}}{{/enumVars}}{{/allowableValues}}
Expand All @@ -30,5 +30,5 @@
@JsonProperty({{{value}}}) {{{name}}}({{{value}}}){{^-last}},{{/-last}}{{#-last}};{{/-last}}
{{/enumVars}}{{/allowableValues}}
}
{{/isEnum}}{{/vars}}{{/hasEnums}}
{{/isPrimitiveType}}{{/isEnum}}{{/vars}}{{/hasEnums}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
{{^isReadOnly}}@get:NotNull{{/isReadOnly}} {{/required}}{{>beanValidationModel}}{{/useBeanValidation}}{{#swaggerAnnotations}}
@ApiModelProperty({{#example}}example = "{{{example}}}", {{/example}}{{#required}}required = {{required}}, {{/required}}{{#isReadOnly}}readOnly = {{{isReadOnly}}}, {{/isReadOnly}}value = "{{{description}}}"){{/swaggerAnnotations}}{{#deprecated}}
@Deprecated(message = ""){{/deprecated}}
@field:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{#isListContainer}}{{baseType}}<{{/isListContainer}}{{classname}}.{{nameInCamelCase}}{{#isListContainer}}>{{/isListContainer}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}? = {{#defaultvalue}}{{defaultvalue}}{{/defaultvalue}}{{^defaultvalue}}null{{/defaultvalue}}
@field:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{^isPrimitiveType}}{{{dataType}}}{{/isPrimitiveType}}{{#isPrimitiveType}}{{#isListContainer}}{{baseType}}<{{/isListContainer}}{{classname}}.{{nameInCamelCase}}{{#isListContainer}}>{{/isListContainer}}{{/isPrimitiveType}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}? = {{#defaultvalue}}{{defaultvalue}}{{/defaultvalue}}{{^defaultvalue}}null{{/defaultvalue}}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{#useBeanValidation}}{{#required}}
{{^isReadOnly}}@get:NotNull{{/isReadOnly}} {{/required}}{{>beanValidationModel}}{{/useBeanValidation}}{{#swaggerAnnotations}}
@ApiModelProperty({{#example}}example = "{{{example}}}", {{/example}}{{#required}}required = {{required}}, {{/required}}{{#isReadOnly}}readOnly = {{{isReadOnly}}}, {{/isReadOnly}}value = "{{{description}}}"){{/swaggerAnnotations}}
@field:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{#isListContainer}}{{baseType}}<{{/isListContainer}}{{classname}}.{{nameInCamelCase}}{{#isListContainer}}>{{/isListContainer}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isReadOnly}}? = {{#defaultvalue}}{{defaultvalue}}{{/defaultvalue}}{{^defaultvalue}}null{{/defaultvalue}}{{/isReadOnly}}
@field:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{^isPrimitiveType}}{{{dataType}}}{{/isPrimitiveType}}{{#isPrimitiveType}}{{#isListContainer}}{{baseType}}<{{/isListContainer}}{{classname}}.{{nameInCamelCase}}{{#isListContainer}}>{{/isListContainer}}{{/isPrimitiveType}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isReadOnly}}? = {{#defaultvalue}}{{defaultvalue}}{{/defaultvalue}}{{^defaultvalue}}null{{/defaultvalue}}{{/isReadOnly}}
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ protected EnumTest() { }
/// <param name="enumInteger">enumInteger.</param>
/// <param name="enumNumber">enumNumber.</param>
/// <param name="outerEnum">outerEnum.</param>
public EnumTest(EnumStringEnum? enumString = default(EnumStringEnum?), EnumStringRequiredEnum enumStringRequired = default(EnumStringRequiredEnum), EnumIntegerEnum? enumInteger = default(EnumIntegerEnum?), EnumNumberEnum? enumNumber = default(EnumNumberEnum?), OuterEnum? outerEnum = default(OuterEnum?))
public EnumTest(EnumStringEnum? enumString = default(EnumStringEnum?), EnumStringRequiredEnum enumStringRequired = default(EnumStringRequiredEnum), EnumIntegerEnum? enumInteger = default(EnumIntegerEnum?), EnumNumberEnum? enumNumber = default(EnumNumberEnum?), OuterEnumEnum? outerEnum = default(OuterEnumEnum?))
{
this.EnumStringRequired = enumStringRequired;
this.EnumString = enumString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ protected EnumTest() { }
/// <param name="enumInteger">enumInteger.</param>
/// <param name="enumNumber">enumNumber.</param>
/// <param name="outerEnum">outerEnum.</param>
public EnumTest(EnumStringEnum? enumString = default(EnumStringEnum?), EnumStringRequiredEnum enumStringRequired = default(EnumStringRequiredEnum), EnumIntegerEnum? enumInteger = default(EnumIntegerEnum?), EnumNumberEnum? enumNumber = default(EnumNumberEnum?), OuterEnum? outerEnum = default(OuterEnum?))
public EnumTest(EnumStringEnum? enumString = default(EnumStringEnum?), EnumStringRequiredEnum enumStringRequired = default(EnumStringRequiredEnum), EnumIntegerEnum? enumInteger = default(EnumIntegerEnum?), EnumNumberEnum? enumNumber = default(EnumNumberEnum?), OuterEnumEnum? outerEnum = default(OuterEnumEnum?))
{
this.EnumStringRequired = enumStringRequired;
this.EnumString = enumString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ protected EnumTest() { }
/// <param name="enumInteger">enumInteger.</param>
/// <param name="enumNumber">enumNumber.</param>
/// <param name="outerEnum">outerEnum.</param>
public EnumTest(EnumStringEnum? enumString = default(EnumStringEnum?), EnumStringRequiredEnum enumStringRequired = default(EnumStringRequiredEnum), EnumIntegerEnum? enumInteger = default(EnumIntegerEnum?), EnumNumberEnum? enumNumber = default(EnumNumberEnum?), OuterEnum? outerEnum = default(OuterEnum?))
public EnumTest(EnumStringEnum? enumString = default(EnumStringEnum?), EnumStringRequiredEnum enumStringRequired = default(EnumStringRequiredEnum), EnumIntegerEnum? enumInteger = default(EnumIntegerEnum?), EnumNumberEnum? enumNumber = default(EnumNumberEnum?), OuterEnumEnum? outerEnum = default(OuterEnumEnum?))
{
// to ensure "enumStringRequired" is required (not null)
if (enumStringRequired == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ data EnumTest = EnumTest
, enumTestEnumStringRequired :: !(E'EnumString) -- ^ /Required/ "enum_string_required"
, enumTestEnumInteger :: !(Maybe E'EnumInteger) -- ^ "enum_integer"
, enumTestEnumNumber :: !(Maybe E'EnumNumber) -- ^ "enum_number"
, enumTestOuterEnum :: !(Maybe OuterEnum) -- ^ "outerEnum"
, enumTestOuterEnum :: !(Maybe E'OuterEnum) -- ^ "outerEnum"
} deriving (P.Show, P.Eq, P.Typeable)

-- | FromJSON EnumTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ enumTestEnumNumberL f EnumTest{..} = (\enumTestEnumNumber -> EnumTest { enumTest
{-# INLINE enumTestEnumNumberL #-}

-- | 'enumTestOuterEnum' Lens
enumTestOuterEnumL :: Lens_' EnumTest (Maybe OuterEnum)
enumTestOuterEnumL :: Lens_' EnumTest (Maybe E'OuterEnum)
enumTestOuterEnumL f EnumTest{..} = (\enumTestOuterEnum -> EnumTest { enumTestOuterEnum, ..} ) <$> f enumTestOuterEnum
{-# INLINE enumTestOuterEnumL #-}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ genEnumTest n =
<*> arbitrary -- enumTestEnumStringRequired :: E'EnumString
<*> arbitraryReducedMaybe n -- enumTestEnumInteger :: Maybe E'EnumInteger
<*> arbitraryReducedMaybe n -- enumTestEnumNumber :: Maybe E'EnumNumber
<*> arbitraryReducedMaybe n -- enumTestOuterEnum :: Maybe OuterEnum
<*> arbitraryReducedMaybe n -- enumTestOuterEnum :: Maybe E'OuterEnum

instance Arbitrary File where
arbitrary = sized genFile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,45 @@ public static EnumNumberEnum fromValue(Double value) {
public static final String JSON_PROPERTY_ENUM_NUMBER = "enum_number";
private EnumNumberEnum enumNumber;

/**
* Gets or Sets outerEnum
*/
public enum OuterEnumEnum {
PLACED("placed"),

APPROVED("approved"),

DELIVERED("delivered");

private OuterEnum value;

OuterEnumEnum(OuterEnum value) {
this.value = value;
}

@JsonValue
public OuterEnum getValue() {
return value;
}

@Override
public String toString() {
return String.valueOf(value);
}

@JsonCreator
public static OuterEnumEnum fromValue(OuterEnum value) {
for (OuterEnumEnum b : OuterEnumEnum.values()) {
if (b.value.equals(value)) {
return b;
}
}
throw new IllegalArgumentException("Unexpected value '" + value + "'");
}
}

public static final String JSON_PROPERTY_OUTER_ENUM = "outerEnum";
private OuterEnum outerEnum;
private OuterEnumEnum outerEnum;


public EnumTest enumString(EnumStringEnum enumString) {
Expand Down Expand Up @@ -295,7 +332,7 @@ public void setEnumNumber(EnumNumberEnum enumNumber) {
}


public EnumTest outerEnum(OuterEnum outerEnum) {
public EnumTest outerEnum(OuterEnumEnum outerEnum) {

this.outerEnum = outerEnum;
return this;
Expand All @@ -310,12 +347,12 @@ public EnumTest outerEnum(OuterEnum outerEnum) {
@JsonProperty(JSON_PROPERTY_OUTER_ENUM)
@JsonInclude(value = JsonInclude.Include.USE_DEFAULTS)

public OuterEnum getOuterEnum() {
public OuterEnumEnum getOuterEnum() {
return outerEnum;
}


public void setOuterEnum(OuterEnum outerEnum) {
public void setOuterEnum(OuterEnumEnum outerEnum) {
this.outerEnum = outerEnum;
}

Expand Down
12 changes: 11 additions & 1 deletion samples/client/petstore/java/google-api-client/docs/EnumTest.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Name | Type | Description | Notes
**enumStringRequired** | [**EnumStringRequiredEnum**](#EnumStringRequiredEnum) | |
**enumInteger** | [**EnumIntegerEnum**](#EnumIntegerEnum) | | [optional]
**enumNumber** | [**EnumNumberEnum**](#EnumNumberEnum) | | [optional]
**outerEnum** | [**OuterEnum**](OuterEnum.md) | | [optional]
**outerEnum** | [**OuterEnumEnum**](#OuterEnumEnum) | | [optional]



Expand Down Expand Up @@ -52,3 +52,13 @@ NUMBER_MINUS_1_DOT_2 | -1.2



## Enum: OuterEnumEnum

Name | Value
---- | -----
PLACED | &quot;placed&quot;
APPROVED | &quot;approved&quot;
DELIVERED | &quot;delivered&quot;



Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,45 @@ public static EnumNumberEnum fromValue(Double value) {
public static final String JSON_PROPERTY_ENUM_NUMBER = "enum_number";
private EnumNumberEnum enumNumber;

/**
* Gets or Sets outerEnum
*/
public enum OuterEnumEnum {
PLACED("placed"),

APPROVED("approved"),

DELIVERED("delivered");

private OuterEnum value;

OuterEnumEnum(OuterEnum value) {
this.value = value;
}

@JsonValue
public OuterEnum getValue() {
return value;
}

@Override
public String toString() {
return String.valueOf(value);
}

@JsonCreator
public static OuterEnumEnum fromValue(OuterEnum value) {
for (OuterEnumEnum b : OuterEnumEnum.values()) {
if (b.value.equals(value)) {
return b;
}
}
throw new IllegalArgumentException("Unexpected value '" + value + "'");
}
}

public static final String JSON_PROPERTY_OUTER_ENUM = "outerEnum";
private OuterEnum outerEnum;
private OuterEnumEnum outerEnum;


public EnumTest enumString(EnumStringEnum enumString) {
Expand Down Expand Up @@ -295,7 +332,7 @@ public void setEnumNumber(EnumNumberEnum enumNumber) {
}


public EnumTest outerEnum(OuterEnum outerEnum) {
public EnumTest outerEnum(OuterEnumEnum outerEnum) {

this.outerEnum = outerEnum;
return this;
Expand All @@ -310,12 +347,12 @@ public EnumTest outerEnum(OuterEnum outerEnum) {
@JsonProperty(JSON_PROPERTY_OUTER_ENUM)
@JsonInclude(value = JsonInclude.Include.USE_DEFAULTS)

public OuterEnum getOuterEnum() {
public OuterEnumEnum getOuterEnum() {
return outerEnum;
}


public void setOuterEnum(OuterEnum outerEnum) {
public void setOuterEnum(OuterEnumEnum outerEnum) {
this.outerEnum = outerEnum;
}

Expand Down
12 changes: 11 additions & 1 deletion samples/client/petstore/java/jersey1/docs/EnumTest.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Name | Type | Description | Notes
**enumStringRequired** | [**EnumStringRequiredEnum**](#EnumStringRequiredEnum) | |
**enumInteger** | [**EnumIntegerEnum**](#EnumIntegerEnum) | | [optional]
**enumNumber** | [**EnumNumberEnum**](#EnumNumberEnum) | | [optional]
**outerEnum** | [**OuterEnum**](OuterEnum.md) | | [optional]
**outerEnum** | [**OuterEnumEnum**](#OuterEnumEnum) | | [optional]



Expand Down Expand Up @@ -52,3 +52,13 @@ NUMBER_MINUS_1_DOT_2 | -1.2



## Enum: OuterEnumEnum

Name | Value
---- | -----
PLACED | &quot;placed&quot;
APPROVED | &quot;approved&quot;
DELIVERED | &quot;delivered&quot;



Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,45 @@ public static EnumNumberEnum fromValue(Double value) {
public static final String JSON_PROPERTY_ENUM_NUMBER = "enum_number";
private EnumNumberEnum enumNumber;

/**
* Gets or Sets outerEnum
*/
public enum OuterEnumEnum {
PLACED("placed"),

APPROVED("approved"),

DELIVERED("delivered");

private OuterEnum value;

OuterEnumEnum(OuterEnum value) {
this.value = value;
}

@JsonValue
public OuterEnum getValue() {
return value;
}

@Override
public String toString() {
return String.valueOf(value);
}

@JsonCreator
public static OuterEnumEnum fromValue(OuterEnum value) {
for (OuterEnumEnum b : OuterEnumEnum.values()) {
if (b.value.equals(value)) {
return b;
}
}
throw new IllegalArgumentException("Unexpected value '" + value + "'");
}
}

public static final String JSON_PROPERTY_OUTER_ENUM = "outerEnum";
private OuterEnum outerEnum;
private OuterEnumEnum outerEnum;


public EnumTest enumString(EnumStringEnum enumString) {
Expand Down Expand Up @@ -295,7 +332,7 @@ public void setEnumNumber(EnumNumberEnum enumNumber) {
}


public EnumTest outerEnum(OuterEnum outerEnum) {
public EnumTest outerEnum(OuterEnumEnum outerEnum) {

this.outerEnum = outerEnum;
return this;
Expand All @@ -310,12 +347,12 @@ public EnumTest outerEnum(OuterEnum outerEnum) {
@JsonProperty(JSON_PROPERTY_OUTER_ENUM)
@JsonInclude(value = JsonInclude.Include.USE_DEFAULTS)

public OuterEnum getOuterEnum() {
public OuterEnumEnum getOuterEnum() {
return outerEnum;
}


public void setOuterEnum(OuterEnum outerEnum) {
public void setOuterEnum(OuterEnumEnum outerEnum) {
this.outerEnum = outerEnum;
}

Expand Down
Loading