-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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.
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
[Java] [Spring] provide optionals also for model fields #1250
Comments
Actually there's a |
Also I don't think it should be mixed with |
What does Optional give when serializing ? |
Agree more fine grained configuration via x- is a good idea. When using Jackson with the java8 module an empty optional will be serialized as null by default so still not what we need. Maybe we need to handle that for the Patch Part manually |
So here are my thoughts:
public class PresenceAware<T> implements Serializable {
private final static PresenceAware<?> ABSENT = new PresenceAware<>(null, false);
private T value;
private boolean isPresent;
private PresenceAware(T value, boolean isPresent) {
this.value = value;
this.isPresent = isPresent;
}
public static <T> PresenceAware<T> absent() {
@SuppressWarnings("unchecked")
PresenceAware<T> t = (PresenceAware<T>) ABSENT;
return t;
}
public static <T> PresenceAware<T> of(T value) {
return new PresenceAware<>(value, true);
}
public T get() {
return value;
}
public boolean isPresent() {
return isPresent;
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof PresenceAware)) {
return false;
}
PresenceAware<?> other = (PresenceAware<?>) obj;
return Objects.equals(value, other.value) &&
Objects.equals(isPresent, other.isPresent);
}
@Override
public int hashCode() {
return Objects.hash(value, isPresent);
}
@Override
public String toString() {
return this.isPresent
? String.format("PresenceAware[%s]", value)
: "PresenceAware.absent";
}
} Then in the DTO private PresenceAware<String> name = PresenceAware.absent();
public Pet name(String name) {
this.name = PresenceAware.of(name);
return this;
}
public PresenceAware<String> getName() {
return name;
}
public void setName(PresenceAware<String> name) {
this.name = name == null ? PresenceAware.absent() : name;
}
@JsonGetter("name")
private Optional<String> _getName() {
return name.isPresent() ? Optional.ofNullable(name.get()) : null;
}
@JsonSetter("name")
private void _setName(String name) {
this.name = PresenceAware.of(name);
} Tests that show it works as expected @RunWith(SpringRunner.class)
@SpringBootTest
public class PetTest {
@Autowired
private ObjectMapper mapper;
@Test
public void sometest() throws IOException {
mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
Pet pet;
String petStr;
pet = mapper.readValue("{\"name\": null}", Pet.class);
assertTrue(pet.getName().isPresent());
assertNull(pet.getName().get());
petStr = mapper.writeValueAsString(pet);
assertTrue(petStr.contains( "\"name\":null"));
pet = mapper.readValue("{}", Pet.class);
assertFalse(pet.getName().isPresent());
assertNull(pet.getName().get());
petStr = mapper.writeValueAsString(pet);
assertFalse(petStr.contains( "\"name\""));
pet = mapper.readValue("{\"name\": \"foo\"}", Pet.class);
assertTrue(pet.getName().isPresent());
pet = new Pet();
assertFalse(pet.getName().isPresent());
assertNull(pet.getName().get());
pet.name("foo");
assertTrue(pet.getName().isPresent());
assertEquals(pet.getName().get(), "foo");
assertEquals(pet.getName().get(), "foo");
petStr = mapper.writeValueAsString(pet);
assertTrue(petStr.contains( "\"name\":\"foo\""));
pet = new Pet();
assertFalse(pet.getName().isPresent());
assertNull(pet.getName().get());
pet.name("foo");
assertTrue(pet.getName().isPresent());
assertEquals(pet.getName().get(), "foo");
}
} Notes:
cc @cowtowncoder (twitter follow-up) |
Looks good I think. How would you model the |
It corresponds to the |
Sounds legit to me, at a high level. One implementation-side thing wrt Jackson support is that this type could probably use much of support that |
Yes. I've begun to look at ReferenceType to write a module but I'm a bit lost... |
@cbornet I think |
@cowtowncoder thanks for your guidance. I could write the
Can you vote for your preferred one or propose a better one ? |
I would prefer JsonNullable. @nullable it also a javax annotation and the use of "Nullable" might cause confusion with that annotation |
I think JsonNullable is perfect. No confusion and not too generic |
Making progress on the module. I have some interrogations about the behavior:
|
First version of the module here : https://github.com/OpenAPITools/jackson-databind-nullable
@cowtowncoder would you help me with that ? (esp. the JsonCreator issue) |
Ok let's see...
So I think you may need to override methods from default/standard reference type deserializer, for (1) and (4). |
Indeed. But
Unfortunately no, it gets handled as |
For 4., |
Ok. Yes, (1) was retrofit to have some way to pre-populate missing entries as Creator must be passed something. As to (4), handling should still be controlled by |
For (1), could For (4), I'm not sure I understand what you recommend. I've tested this: @Override
public JsonNullable<Object> deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
JsonToken t = p.getCurrentToken();
if (t == JsonToken.VALUE_STRING && !_fullType.isTypeOrSubTypeOf(String.class)) {
String str = p.getText().trim();
if (str.isEmpty()) {
return JsonNullable.undefined();
}
}
return super.deserialize(p, ctxt);
} which seems to work. Is it the correct way to do it ? |
Forget my comment about |
* Use JsonNullable wrapper on nullable/x-nullable fields Fix #1250 * update samples
…1762) * Use JsonNullable wrapper on nullable/x-nullable fields Fix OpenAPITools#1250 * update samples
Description
Follow up of discussion with @cbornet https://twitter.com/atomfrede/status/1051787697201827840
When setting
useOptional=true
only query parameter are affected. All model fields are are not affected.Why is that useful? When using json-patch-merge a field not send is different from a value explicitly set to null. Where in the first case the field should not be updated in the second case it means the field should be removed. Currently this is not possible as both a not provided field and field set to null will result in the field being null. When the model fields are optional a field being null would mean it was not send and an empty optional would mean it was send but set to null.
Example
{a:b,c:d}
-- Patch/Merge{c:null}
-->{a:b}
(c is removed){a:b,c:d}
-- Patch/Merge{c:""}
-->{a:b,c:""}
(c is set to emtpy string)openapi-generator version
3.3.0
Related issues/PRs
Couldn't find any.
Suggest a fix/enhancement
Optional fields (or maybe optional fields when using patch) and setting
useOptional=true
should make a difference between value not set (==null) and value set to null (==empty optional) to support json-patch-mergeThe text was updated successfully, but these errors were encountered: