Fix #3428 Fixed subtype hierarchy search and property names #3429
Conversation
|
|
There was a problem hiding this comment.
Hi @FatalCatharsis,
Thank you for contributing this. It is really appreciated.
Before merging, I'm asking you to make some minor changes to better match existing code.
Also, please run mvn formatter:format before committing, as otherwise the PR check will fail.
| import io.swagger.v3.oas.models.media.Schema; | ||
| import io.swagger.v3.oas.models.media.StringSchema; | ||
|
|
||
| import org.apache.commons.lang3.StringUtils; |
There was a problem hiding this comment.
lang3 is not a direct dependency in Hilla. If possible, avoid using it.
| .stream().flatMap(Arrays::stream); | ||
| } | ||
|
|
||
| private static Optional<Pair<JsonTypeInfo, JsonSubTypes.Type[]>> getJsonSubTypeInHierarchy( |
There was a problem hiding this comment.
You can use a record for better readability and to avoid using Pair.
| package com.vaadin.hilla.parser.plugins.subtypes; | ||
|
|
||
| public class AdvancedAddEvent extends AddEvent { | ||
| private Boolean defer; |
There was a problem hiding this comment.
Private fields are not generated, so this field is probably useless in the test. You could make it public to match the other examples.
| "info": { | ||
| "title": "Hilla Application", | ||
| "version": "1.0.0" | ||
| "openapi" : "3.0.1", |
There was a problem hiding this comment.
| "openapi" : "3.0.1", | |
| "openapi": "3.0.1", |
In those files, remove the spaces before colons to reduce the number of changes in the PR.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3429 +/- ##
=======================================
Coverage 86.68% 86.68%
=======================================
Files 115 115
Lines 8195 8195
Branches 1272 1272
=======================================
Hits 7104 7104
Misses 1075 1075
Partials 16 16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Fix #3428 find JsonSubTypes on supertype hierarchy and use specified property name
Description
Recurses up through all superclasses and finds the first instance of JsonSubTypes to use, instead of looking only at the direct ancestor. Also uses the property on the related JsonTypeInfo, instead of assuming it's the default
@typeproperty.Just reported this today and figured I'd spend an hour or so since the fix was really obvious. A more aggressive change I kind of wanted to make was replacing the polymorphic type on the endpoint method with the created union type, but I couldn't grok the plugin logic enough to work out if that was possible, let me know if that's desirable and possible.
note: the provided eclipse settings file made a couple whitespace changes, so I'm thinking the file was made before you had the eclipse style xml. Either which way, I promise I applied it :)
Fixes #3428
Type of change
Checklist