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

[OpenAPI 3.1] Avoid NPE when handling prefixItems #19735

Merged
merged 2 commits into from
Oct 1, 2024
Merged

[OpenAPI 3.1] Avoid NPE when handling prefixItems #19735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a83fecf
avoid npe when handling prefixItems in 3.1 spec
wing328 Oct 1, 2024
29ba171
update samples
wing328 Oct 1, 2024
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
3 changes: 3 additions & 0 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,9 @@ private Schema processNormalize31Spec(Schema schema, Set<Schema> visitedSchemas)
Schema updatedItems = normalizeSchema(schema.getItems(), visitedSchemas);
as.setItems(updatedItems);
}
} else {
// when items is not defined, default to any type
as.setItems(new Schema());
}

return as;
Expand Down
10 changes: 10 additions & 0 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,11 @@ public void testOpenAPINormalizerProcessingArraySchema31Spec() {
assertEquals(((Schema) schema5.getProperties().get("arrayOfStrings")).getItems().getType(), null);
assertEquals(((Schema) schema5.getProperties().get("arrayOfStrings")).getItems().getTypes().contains("string"), true);

Schema schema7 = openAPI.getComponents().getSchemas().get("ArrayWithPrefixItems");
assertEquals(((Schema) schema7.getProperties().get("with_prefixitems")).getItems(), null);
assertNotEquals(((Schema) schema7.getProperties().get("with_prefixitems")).getPrefixItems(), null);
assertEquals(((Schema) schema7.getProperties().get("without_items")).getItems(), null);

Map<String, String> inputRules = Map.of("NORMALIZE_31SPEC", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, inputRules);
openAPINormalizer.normalize();
Expand All @@ -622,6 +627,11 @@ public void testOpenAPINormalizerProcessingArraySchema31Spec() {
assertEquals(((Schema) schema6.getProperties().get("arrayOfStrings")).getItems().getTypes().contains("string"), true);
assertEquals(((Schema) schema6.getProperties().get("arrayOfStrings")).getItems().getType(), "string");
assertEquals(((Schema) schema6.getProperties().get("arrayOfStrings")).getType(), "array");

Schema schema8 = openAPI.getComponents().getSchemas().get("ArrayWithPrefixItems");
assertNotEquals(((Schema) schema8.getProperties().get("with_prefixitems")).getItems(), null);
Copy link
Contributor

Choose a reason for hiding this comment

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

OOI, why not:

Suggested change
assertNotEquals(((Schema) schema8.getProperties().get("with_prefixitems")).getItems(), null);
assertNotNull(((Schema) schema8.getProperties().get("with_prefixitems")).getItems());

?

Copy link
Member Author

Choose a reason for hiding this comment

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

to me, it's much easier to read when comparing 2 values

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Interesting, for me it's the opposite, scanning the assertion function first. Also, some of the convenience functions have more precise error messages in case the assertion fails, which I always appreciate whilst sorting out any messes I may have made :)

assertEquals(((Schema) schema8.getProperties().get("with_prefixitems")).getPrefixItems(), null);
assertNotEquals(((Schema) schema8.getProperties().get("without_items")).getItems(), null);
}

@Test
Expand Down
17 changes: 15 additions & 2 deletions modules/openapi-generator/src/test/resources/3_1/issue_18291.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,20 @@ components:
type: array
items:
type: string

Bar:
allOf:
- $ref: '#/components/schemas/Foo'
- $ref: '#/components/schemas/Foo'
ArrayWithPrefixItems:
type: object
properties:
with_prefixitems:
type: array
prefixItems:
- type: number
title: Longitude
- type: number
title: Latitude
maxItems: 2
minItems: 2
without_items:
type: array
8 changes: 2 additions & 6 deletions samples/client/petstore/java/okhttp-gson-3.1/api/openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,9 +1099,7 @@ components:
ref_array_prefix_items:
description: |
An item that was added to the queue.
items:
description: TODO default missing array inner type to string
type: string
items: {}
maxItems: 5
minItems: 3
type: array
Expand All @@ -1112,9 +1110,7 @@ components:
ArrayPrefixItems:
description: |
An item that was added to the queue.
items:
description: TODO default missing array inner type to string
type: string
items: {}
maxItems: 5
minItems: 3
type: array
Expand Down
2 changes: 1 addition & 1 deletion samples/client/petstore/java/okhttp-gson-3.1/docs/AnyTypeTest.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
|------------ | ------------- | ------------- | -------------|
|**anyTypeProperty** | **Object** | | [optional] |
|**arrayProp** | **List&lt;String&gt;** | test array in 3.1 spec | [optional] |
|**refArrayPrefixItems** | **List&lt;String&gt;** | An item that was added to the queue. | [optional] |
|**refArrayPrefixItems** | **List&lt;Object&gt;** | An item that was added to the queue. | [optional] |



10 changes: 5 additions & 5 deletions ...etstore/java/okhttp-gson-3.1/src/main/java/org/openapitools/client/model/AnyTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public class AnyTypeTest {

public static final String SERIALIZED_NAME_REF_ARRAY_PREFIX_ITEMS = "ref_array_prefix_items";
@SerializedName(SERIALIZED_NAME_REF_ARRAY_PREFIX_ITEMS)
private List<String> refArrayPrefixItems = new ArrayList<>();
private List<Object> refArrayPrefixItems = new ArrayList<>();

public AnyTypeTest() {
}
Expand Down Expand Up @@ -114,12 +114,12 @@ public void setArrayProp(List<String> arrayProp) {
}


public AnyTypeTest refArrayPrefixItems(List<String> refArrayPrefixItems) {
public AnyTypeTest refArrayPrefixItems(List<Object> refArrayPrefixItems) {
this.refArrayPrefixItems = refArrayPrefixItems;
return this;
}

public AnyTypeTest addRefArrayPrefixItemsItem(String refArrayPrefixItemsItem) {
public AnyTypeTest addRefArrayPrefixItemsItem(Object refArrayPrefixItemsItem) {
if (this.refArrayPrefixItems == null) {
this.refArrayPrefixItems = new ArrayList<>();
}
Expand All @@ -132,11 +132,11 @@ public AnyTypeTest addRefArrayPrefixItemsItem(String refArrayPrefixItemsItem) {
* @return refArrayPrefixItems
*/
@javax.annotation.Nullable
public List<String> getRefArrayPrefixItems() {
public List<Object> getRefArrayPrefixItems() {
return refArrayPrefixItems;
}

public void setRefArrayPrefixItems(List<String> refArrayPrefixItems) {
public void setRefArrayPrefixItems(List<Object> refArrayPrefixItems) {
this.refArrayPrefixItems = refArrayPrefixItems;
}

Expand Down
16 changes: 8 additions & 8 deletions ...lient/petstore/rust/reqwest/petstore-async-tokensource/src/models/numeric_enum_testing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ pub enum NumericEnumTesting {

}

impl ToString for NumericEnumTesting {
Copy link
Contributor

Choose a reason for hiding this comment

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

refs #19736

fn to_string(&self) -> String {
match self {
Self::Variant0 => String::from("0"),
Self::Variant1 => String::from("1"),
Self::Variant2 => String::from("2"),
Self::Variant3 => String::from("3"),
}
impl std::fmt::Display for NumericEnumTesting {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", match self {
Self::Variant0 => "0",
Self::Variant1 => "1",
Self::Variant2 => "2",
Self::Variant3 => "3",
})
}
}
impl Default for NumericEnumTesting {
Expand Down
Loading