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.

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

FIX: mutationDetection = SOURCE works also for special json types #2309

Merged

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Aug 13, 2021

Hello Rob, I discovered a bug in the new mutationDetection.

While

@DbSource(mutationDetection = SOURCE)
List<PlainBean> plainBeanList;

works well, it does not work for

@DbSource(mutationDetection = SOURCE)
List<String> stringList;

because here Ebean uses ScalarTypeJsonList

I tried to write a test for all @DbJson annotations. The same problem would occur with @DbArray (which we did not use)
We currently use a lot of @DbJson annotations and we rely on a proper dirtyValue (oldValue<->newValue) recognition.

cheers
Roland

}

@Override
public boolean isJsonMapper() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

checkme: Should we refactor that getter to isKeepSource pushSource / popSource

Copy link
Member

Choose a reason for hiding this comment

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

Review this ...

@@ -71,13 +71,25 @@ public JsonB(DocPropertyType docPropertyType, boolean nullable) {
@SuppressWarnings("rawtypes")
private abstract static class Base extends ScalarTypeJsonCollection<Set> {

public Base(int dbType, DocPropertyType docPropertyType, boolean nullable) {
private boolean keepSource;
Copy link
Member

Choose a reason for hiding this comment

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

final

return null;
}
try {
try (InputStreamReader inputStreamReader = new InputStreamReader(is)) {
return parse(inputStreamReader);
if (isJsonMapper()) {
Copy link
Member

Choose a reason for hiding this comment

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

review and formatting

@@ -26,88 +27,95 @@
@SuppressWarnings("rawtypes")
public abstract class ScalarTypeJsonMap extends ScalarTypeBase<Map> {

private static final ScalarTypeJsonMap CLOB = new ScalarTypeJsonMap.Clob();
Copy link
Member

Choose a reason for hiding this comment

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

Worth keeping?

}

@Override
public boolean isJsonMapper() {
Copy link
Member

Choose a reason for hiding this comment

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

Review this ...

} else {
return createJsonObjectMapperType(prop, dbType, DocPropertyType.OBJECT);
}
}
if (objectMapperPresent) {
if (objectMapperPresent && prop.getMutationDetection() == MutationDetection.DEFAULT) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we just remove this altogether ?

@rbygrave rbygrave merged commit e2b7434 into ebean-orm:master Aug 15, 2021
@rbygrave rbygrave added this to the 12.11.1 milestone Aug 15, 2021
@rbygrave rbygrave added the bug label Aug 15, 2021
@rPraml rPraml deleted the mutationdetection-source-special-json-types branch February 25, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants