Skip to content

Commit

Permalink
[Java][bugfix] hashcode/equals behave incorrectly when inheritance is…
Browse files Browse the repository at this point in the history
… used in Generated Pojos (#15745)

* Fix Java equals and hashCode methods to work with inheritence (#5756)

* Regenerate code samples with improved hashCode/equals

---------

Co-authored-by: Andrew Pikler <[email protected]>
  • Loading branch information
pyckle and Andrew Pikler authored Jun 21, 2023
1 parent 7e89e1e commit 33aa5b0
Show file tree
Hide file tree
Showing 266 changed files with 1,050 additions and 1,017 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,13 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,13 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{/vars}}{{#parent}} &&
super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}}{{#parent}}{{#hasVars}}, {{/hasVars}}super.hashCode(){{/parent}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,24 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}}{{#vendorExtensi

{{/vars}}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,15 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
}
if (o == null || getClass() != o.getClass()) {
return false;
}{{#hasVars}}
{{classname}} {{classVarName}} = ({{classname}}) o;
return {{#vars}}Objects.equals(this.{{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{/vars}}{{#parent}} &&
super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}}{{#parent}}{{#hasVars}}, {{/hasVars}}super.hashCode(){{/parent}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,13 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,13 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,13 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,13 @@ public class {{classname}} {{#parent}}extends {{{.}}}{{/parent}} {{#vendorExtens
return false;
}
{{classname}} {{classVarName}} = ({{classname}}) o;{{#hasVars}}
return {{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}
return true;{{/hasVars}}
return {{#parent}}super.equals(o) && {{/parent}}{{#vars}}Objects.equals({{name}}, {{classVarName}}.{{name}}){{^-last}} &&
{{/-last}}{{#-last}};{{/-last}}{{/vars}}{{/hasVars}}{{^hasVars}}{{#parent}}return super.equals(o);{{/parent}}{{^parent}}return true;{{/parent}}{{/hasVars}}
}

@Override
public int hashCode() {
return Objects.hash({{#vars}}{{name}}{{^-last}}, {{/-last}}{{/vars}});
return {{^hasVars}}{{#parent}}super.hashCode(){{/parent}}{{^parent}}1{{/parent}}{{/hasVars}}{{#hasVars}}Objects.hash({{#vars}}{{#parent}}super.hashCode(), {{/parent}}{{name}}{{^-last}}, {{/-last}}{{/vars}}){{/hasVars}};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.openapitools.codegen.DefaultGenerator;
import org.openapitools.codegen.MockDefaultGenerator;
import org.openapitools.codegen.TestUtils;
import org.openapitools.codegen.java.assertions.JavaFileAssert;
import org.openapitools.codegen.languages.AbstractJavaJAXRSServerCodegen;
import org.openapitools.codegen.languages.features.CXFServerFeatures;
import org.testng.Assert;
Expand All @@ -21,6 +22,8 @@
import java.util.List;
import java.util.Map;
import java.nio.file.Paths;
import java.util.function.Function;
import java.util.stream.Collectors;

import static org.openapitools.codegen.TestUtils.assertFileContains;
import static org.openapitools.codegen.TestUtils.assertFileNotContains;
Expand Down Expand Up @@ -300,4 +303,35 @@ public void testExtraAnnotations() throws IOException {
TestUtils.assertExtraAnnotationFiles(outputPath + "/src/gen/java/org/openapitools/model");

}

@Test(description = "Validate that the generated equals()/hashCode() methods call super.equals() and super.hasCode()")
public void testClassInheritanceEqualsHashCode() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

codegen.setOutputDir(output.getAbsolutePath());

OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/allOf_no_fields.yaml");
ClientOptInput input = new ClientOptInput()
.openAPI(openAPI)
.config(codegen);

DefaultGenerator generator = new DefaultGenerator();
Map<String, File> files = generator.opts(input)
.generate().stream()
.collect(Collectors.toMap(File::getName, Function.identity()));

// Assert that the base class does not call super.equals() or super.hashCode()
JavaFileAssert.assertThat(files.get("BaseClass.java")).assertMethod("equals").bodyNotContainsLines("super");
JavaFileAssert.assertThat(files.get("BaseClass.java")).assertMethod("hashCode").bodyNotContainsLines("super");

// Assert that the child class does call the super.equals and super.hashCode method
assertCallsSuperInEqualsAndHashcode(files.get("ChildWithProperties.java"));
assertCallsSuperInEqualsAndHashcode(files.get("ChildWithoutProperties.java"));
}

private static void assertCallsSuperInEqualsAndHashcode(File toCheck) {
JavaFileAssert.assertThat(toCheck).assertMethod("equals").bodyContainsLines("super.equals");
JavaFileAssert.assertThat(toCheck).assertMethod("hashCode").bodyContainsLines("super.hashCode");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
openapi: 3.0.1
info:
version: 1.0.0
title: Example
license:
name: MIT
servers:
- url: http://api.example.xyz/v1
paths:
/example:
get:
operationId: dummyid
responses:
'200':
description: OK
content:
application/json:
schema:
$ref: "#/components/schemas/BaseClass"
components:
schemas:
BaseClass:
type: object
properties:
myDiscrim:
type: string
discriminator:
propertyName: myDiscrim
mapping:
WITHPROPS: '#/components/schemas/ChildWithProperties'
WITHOUTPROPS: '#/components/schemas/ChildWithoutProperties'
ChildWithoutProperties:
type: object
allOf:
- $ref: '#/components/schemas/BaseClass'
ChildWithProperties:
type: object
allOf:
- type: object
properties:
childProperty:
type: string
- $ref: '#/components/schemas/BaseClass'
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesAnyType additionalPropertiesAnyType = (AdditionalPropertiesAnyType) o;
return Objects.equals(this.name, additionalPropertiesAnyType.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesAnyType.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesArray additionalPropertiesArray = (AdditionalPropertiesArray) o;
return Objects.equals(this.name, additionalPropertiesArray.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesArray.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesBoolean additionalPropertiesBoolean = (AdditionalPropertiesBoolean) o;
return Objects.equals(this.name, additionalPropertiesBoolean.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesBoolean.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,17 +383,17 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesClass additionalPropertiesClass = (AdditionalPropertiesClass) o;
return Objects.equals(this.mapString, additionalPropertiesClass.mapString) &&
Objects.equals(this.mapNumber, additionalPropertiesClass.mapNumber) &&
Objects.equals(this.mapInteger, additionalPropertiesClass.mapInteger) &&
Objects.equals(this.mapBoolean, additionalPropertiesClass.mapBoolean) &&
Objects.equals(this.mapArrayInteger, additionalPropertiesClass.mapArrayInteger) &&
Objects.equals(this.mapArrayAnytype, additionalPropertiesClass.mapArrayAnytype) &&
Objects.equals(this.mapMapString, additionalPropertiesClass.mapMapString) &&
Objects.equals(this.mapMapAnytype, additionalPropertiesClass.mapMapAnytype) &&
Objects.equals(this.anytype1, additionalPropertiesClass.anytype1) &&
Objects.equals(this.anytype2, additionalPropertiesClass.anytype2) &&
Objects.equals(this.anytype3, additionalPropertiesClass.anytype3);
return Objects.equals(mapString, additionalPropertiesClass.mapString) &&
Objects.equals(mapNumber, additionalPropertiesClass.mapNumber) &&
Objects.equals(mapInteger, additionalPropertiesClass.mapInteger) &&
Objects.equals(mapBoolean, additionalPropertiesClass.mapBoolean) &&
Objects.equals(mapArrayInteger, additionalPropertiesClass.mapArrayInteger) &&
Objects.equals(mapArrayAnytype, additionalPropertiesClass.mapArrayAnytype) &&
Objects.equals(mapMapString, additionalPropertiesClass.mapMapString) &&
Objects.equals(mapMapAnytype, additionalPropertiesClass.mapMapAnytype) &&
Objects.equals(anytype1, additionalPropertiesClass.anytype1) &&
Objects.equals(anytype2, additionalPropertiesClass.anytype2) &&
Objects.equals(anytype3, additionalPropertiesClass.anytype3);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesInteger additionalPropertiesInteger = (AdditionalPropertiesInteger) o;
return Objects.equals(this.name, additionalPropertiesInteger.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesInteger.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesNumber additionalPropertiesNumber = (AdditionalPropertiesNumber) o;
return Objects.equals(this.name, additionalPropertiesNumber.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesNumber.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesObject additionalPropertiesObject = (AdditionalPropertiesObject) o;
return Objects.equals(this.name, additionalPropertiesObject.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesObject.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ public boolean equals(Object o) {
return false;
}
AdditionalPropertiesString additionalPropertiesString = (AdditionalPropertiesString) o;
return Objects.equals(this.name, additionalPropertiesString.name) &&
super.equals(o);
return super.equals(o) && Objects.equals(name, additionalPropertiesString.name);
}

@Override
public int hashCode() {
return Objects.hash(name, super.hashCode());
return Objects.hash(super.hashCode(), name);
}

@Override
Expand Down
Loading

0 comments on commit 33aa5b0

Please sign in to comment.