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.

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

feat(openapi-generator): Suppress faulty additionalProperties: true #689

Merged
merged 12 commits into from
Jan 29, 2025
Merged

feat(openapi-generator): Suppress faulty additionalProperties: true #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8155673
Additional unit test checking for serialization of Order
a-d Jan 10, 2025
cb7a250
Merge remote-tracking branch 'origin/main' into openapi/fix-additonal…
a-d Jan 10, 2025
7bd61ed
Initial problem
a-d Jan 10, 2025
da03cb2
Add fix
a-d Jan 10, 2025
ce8583b
Apply fix
a-d Jan 10, 2025
626a111
Fix PMD warnings
a-d Jan 10, 2025
7d10987
Merge remote-tracking branch 'origin/main' into openapi/fix-additonal…
a-d Jan 29, 2025
eb0ead1
Update
a-d Jan 29, 2025
00a3bec
Fix PMD finding
a-d Jan 29, 2025
de634ef
Add test assertion
a-d Jan 29, 2025
b64c696
Add release note
a-d Jan 29, 2025
1a5e417
Update datamodel/openapi/openapi-api-sample/pom.xml
MatKuhr Jan 29, 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
1 change: 1 addition & 0 deletions datamodel/openapi/openapi-api-sample/src/main/resources/sodastore.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ components:
example: 123
Order:
type: object
additionalProperties: true
required:
- productId
- quantity
Expand Down
23 changes: 22 additions & 1 deletion ...ample/src/test/java/com/sap/cloud/sdk/datamodel/openapi/sample/api/SerializationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
import com.sap.cloud.sdk.cloudplatform.connectivity.DefaultHttpDestination;
import com.sap.cloud.sdk.datamodel.openapi.sample.model.Order;
import com.sap.cloud.sdk.datamodel.openapi.sample.model.SodaWithId;

@WireMockTest
Expand Down Expand Up @@ -48,7 +49,7 @@ void testPutPayload()
}

@Test
void testJacksonSerialization()
void testJacksonSerializeSodaWithId()
throws JsonProcessingException
{
expected = """
Expand All @@ -75,6 +76,26 @@ void testJacksonSerialization()
assertThat(new ObjectMapper().writeValueAsString(obj)).isEqualToIgnoringWhitespace(expected);
}

@Test
void testJacksonSerializeOrder()
newtork marked this conversation as resolved.
Show resolved Hide resolved
throws JsonProcessingException
{
expected = """
{
"productId": 100,
"quantity": 5,
"totalPrice": 6.0,
"typelessProperty":null,
"nullableProperty":null,
"shoesize": 44
}
""";
final Order order = Order.create().productId(100L).quantity(5).totalPrice(6.0f);
order.setCustomField("shoesize", 44);
assertThat(new ObjectMapper().writeValueAsString(order)).isEqualToIgnoringWhitespace(expected);
Copy link
Contributor Author

@newtork newtork Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

(Comment)

Without the enabled feature toggle this test assertion fails:

Expecting actual:
  "{}"
to be equal to:
  "{
  "productId": 100,
  "quantity": 5,
  "totalPrice": 6.0,
  "packaging" : "bottle",
  "shoesize": 44
}

assertThat(new ObjectMapper().readValue(expected, Order.class)).isEqualTo(order);
}

private void verify( String requestBody )
{
WireMock
Expand Down
12 changes: 12 additions & 0 deletions .../java/com/sap/cloud/sdk/datamodel/openapi/generator/GenerationConfigurationConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import org.openapitools.codegen.ClientOptInput;
import org.openapitools.codegen.CodegenConstants;
import org.openapitools.codegen.CodegenModel;
import org.openapitools.codegen.CodegenOperation;
import org.openapitools.codegen.config.GlobalSettings;
import org.openapitools.codegen.languages.JavaClientCodegen;
Expand All @@ -23,6 +24,7 @@

import io.swagger.parser.OpenAPIParser;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.parser.core.models.AuthorizationValue;
import io.swagger.v3.parser.core.models.ParseOptions;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -89,6 +91,16 @@ public OperationsMap postProcessOperationsWithModels(
}
return super.postProcessOperationsWithModels(ops, allModels);
}

@SuppressWarnings( { "rawtypes", "RedundantSuppression" } )
@Override
protected void updateModelForObject( @Nonnull final CodegenModel m, @Nonnull final Schema schema )
{
// Disable additional attributes to prevent model classes from extending "HashMap"
// SAP Cloud SDK offers custom field APIs to handle additional attributes already
schema.setAdditionalProperties(Boolean.FALSE);
super.updateModelForObject(m, schema);
}
};
}

Expand Down
3 changes: 2 additions & 1 deletion release_notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@

### 🐛 Fixed Issues

-
- Fix non-compilable code using OpenAPI generator with schema definitions having `additionalProperties: true`.
Previously they would result in model classes extending `HashMap`, which disabled proper deserialization and serialization.
Loading