feat: Add JsonPropertyOrder annotation to all classes #2820
Conversation
|
Can one of the admins verify this patch? |
|
Things I would like reviewed:
|
samuel-hawker
force-pushed
the
2779-validate-json-property-order-present
branch
from
485bb7a to
c56f30a
Compare
|
@samuel-hawker This looks good to me. But having feedback from Tom would be good. But I think he is off this week, so we might need to wait until next week for him. |
| @@ -427,9 +427,16 @@ private void checkClassOverrides(Class<?> crdClass, String methodName, Class<?>. | |||
| } | |||
| result.putAll(properties(crdClass)); | |||
| JsonPropertyOrder order = crdClass.getAnnotation(JsonPropertyOrder.class); | |||
| if (order == null && !isExemptClass(crdClass)) { | |||
| throw new InvalidCrdException(crdClass.getName() + " missing @JsonPropertyOrder annotation"); | |||
There was a problem hiding this comment.
I think you just need to call err() (which will log the error and exit at the end, that way you get told about all the errors, rather than having to fix them one at a time).
| return sortedProperties(order != null ? order.value() : null, result).values(); | ||
| } | ||
|
|
||
| private boolean isExemptClass(Class<?> crdClass) { |
There was a problem hiding this comment.
This method name is too generic. There's plenty of checks on classes in the CrdGenerator, so you need to give it a name which makes clear what it's exempt from. Maybe inverting it and calling it requiresPropertyOrder(Class<?>)?
Stage 1 of Issue 2779 Validate @JsonPropertyOrder present for all CRD classes and subclasses with exception of Java primitive `Object` Added test class to validate this new functionality and added Exception: InvalidCrdException Followup PRs will see the check that JsonPropertyOrder contains exactly all of the classes fields and these fields will be ordered for each individual class Contributes to: strimzi#2779 Signed-off-by: Samuel Hawker <samuel.hawker@ibm.com>
Signed-off-by: Samuel Hawker <samuel.hawker@ibm.com>
samuel-hawker
force-pushed
the
2779-validate-json-property-order-present
branch
from
c56f30a to
dddbb2f
Compare
|
Still ironing out some problems with the tests, closing for awhile |
Type of change
Select the type of your PR
Description
Stage 1 of Issue 2779
Validate @JsonPropertyOrder present for all CRD classes
and subclasses with exception of Java primitive
ObjectAdded test class to validate this new functionality and
added Exception: InvalidCrdException
Followup PRs will see the check that JsonPropertyOrder
contains exactly all of the classes fields and these fields
will be ordered for each individual class
Contributes to: #2779
Signed-off-by: Samuel Hawker samuel.hawker@ibm.com
Checklist
Please go through this checklist and make sure all applicable tasks have been done
./design