Closed shawkins closed 1 year ago
The bug might be legit, have you attempted to explicitly add @JsonIgnore
as a temporary workaround to values
?
It would be great to reproduce it in the test suite.
have you attempted to explicitly add @JsonIgnore as a temporary workaround to values
Where would I add that on an enum?
It would be great to reproduce it in the test suite.
The stack overflow post also complains about the unpredictable nature of seeing both representations.
Where would I add that on an enum?
If I for example add an explicit ENUM$VALUES field or enum value with a jsonignore, then when this issue is occurring another ENUM$VALUES_0 synthetic field will be added automatically. There doesn't seem to be a way to apply an annotation to synthethic field.
A couple of additional observations:
Thanks a lot for the additional information @shawkins ! I'll take a closer look into this early next week, I hope that sundrio
exposes the API to identify synthetic fields 🤞 otherwise is going to be a much slower fix.
In the test suite we are already generating the Realm import CRD from Keycloak, have you already verified if the bug reproduces locally to this codebase?
I hope that sundrio exposes the API to identify synthetic fields crossed_fingers otherwise is going to be a much slower fix.
Unfortunately it does not - it probably should to then not use starting with $ as a filter. However it does convey that the field is static and private - that should not be included with the rest of the enums. Adding a filter like:
.filter(p -> !p.isStatic() || !p.isPrivate())
Will address this specific case.
It does bring up that only simple enums are supported - if you also include state in the enum, then it will be mistakenly included in the schema:
public enum Category {
Any,
Misc,
Programming,
Dark,
Pun,
Spooky,
Christmas;
private int rating = 1;
public int getRating() {
return rating;
}
}
causes rating to show up as an enum value. Maybe only public static fields should be included (it is of course possible with jackson to change the serialization of enums https://www.baeldung.com/jackson-serialize-enums but none of that is supported either).
In the test suite we are already generating the Realm import CRD from Keycloak, have you already verified if the bug reproduces locally to this codebase?
I was able to reproduce it eventually just with the JokeRequestSpec - ENUM$VALUES showed up as an enum value. However I still don't know what causes that. Trying different jres, etc., doesn't seem to guarentee that it will happen - it didn't initially happen when testing the SchemaSwap either.
Whatever causes it, it is known as a possibility in the jdk source: https://github.com/openjdk/jdk11u/blob/1000b1c0074af4b954c5f210d1684b5623e9723b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/spi/JavaConstantFieldProvider.java#L131
This issue might have the same solution as: #4225
@andreaTP for now let's combine the filtering from that pr with the public static requirement:
.filter(p -> p.isStatic() && p.isPublic() && def.getFullyQualifiedName().equals(p.getTypeRef().toString()))
That will leave the corner case of a public static field on the enum that is typed the same as the enum (should be extremely rare). The only way to account for that is to expand sundrio to expose Field.isEnumConstant() - since that's such a corner case though I'd vote on adding the filtering now rather than waiting for the sundrio fix.
@shawkins I agree that we need to get a fix for this.
should be extremely rare
This is the only detail I disagree with since a getDefault()
method in enums is pretty common.
Still, I don't want to get stuck on this, let's time-box the "proper" fix, if we can get a new sundrio
release including the surfacing for isEnumConstant
and isSynthetic
within a week with the help of @iocanel and @metacosm it would be great.
Meanwhile, if you want to submit the proposed workaround I'll accept it to make sure we improve for the next release.
This is the only detail I disagree with since a getDefault() method in enums is pretty common.
This would have to be expressed as:
enum Something {
VALUE;
public static final Something DEFAULT_VALUE = VALUE;
}
Using a method would be fine because we're only looking at the fields.
Meanwhile, if you want to submit the proposed workaround I'll accept it to make sure we improve for the next release.
Marking resolved by #4230 - there will be another improvement after https://github.com/sundrio/sundrio/pull/389
Describe the bug
There seems to be some inconsistency with the VM reflection reporting $VALUES vs ENUM$VALUES - https://stackoverflow.com/questions/46924470/java-enums-enumvalues-or-values
If ENUM$VALUES is reported - and I don't quite see why it is as the same jre gives me different behavior at times - then it won't be filtered by the logic looking for a name starting with a $ - https://github.com/fabric8io/kubernetes-client/blob/683336b86ce53d07c2986068288f3de627c24536/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java#L668
Fabric8 Kubernetes Client version
6.6.2
Steps to reproduce
Not quite sure the behavior seems inconsistent.
Expected behavior
That the VALUES field will be omitted regardless.
Ideally ClassToTypeDef would check / filter synthetic properties rather than just relying upon the $ convention.
Runtime
other (please specify in additional context)
Kubernetes API Server version
other (please specify in additional context)
Environment
Linux
Fabric8 Kubernetes Client Logs
No response
Additional context
No response