Change DefaultObjectMapper to NOT overwrite final fields unless explicitly asked to

[drcrallen]

https://groups.google.com/d/msg/jackson-user/1EFZzvhu_j0/NnDNahOZFAAJ

Turns out Jackson will very happily auto-detect final fields and overwrite them. This was causing a few bugs in how things were named to go unnoticed.

This is labeled as Discuss because it breaks backwards compatibility, and is intended to show a few locations where naming errors exist. If it is deemed we should fix this behavior I can add backwards compatibility.

If accepted there is a risk that some POJOs which do not have proper serde tests may break.

Another option is to leave the behavior as is, and simply have it generally be known that this behavior exists in Jackson.

[drcrallen]

BrokerServerViewTest.testSingleServerAddedRemovedSegment:113->setupZNodeForServer:384 » NodeExists

[guobingkun]

From the doc, realtime ingestion is using maxRowsInMemory and batch ingestion is using rowFlushBoundary, though in the code they share the same field, is it intentional or simply because we want backward compatibility?

[fjy]

@guobingkun both processes should use the same config and the docs shoudl be changed to reflect this. 0.9.0 is a good chance to make this change.

[guobingkun]

@fjy 👍

[himanshug]

this solves some but not actually all the problem, say the field "rowFlushBoundary" in HadoopTuningConfig wasn't a final field, then the test will still pass which is again weird (it happens because jackson auto detected the field)
however, i believe, we should add this as it takes away at least some unintended magic if not all.

[drcrallen]

My reason for wanting this config is because if a field is assumed final and the constructor uses it, then it is potentially later set to something else, that is an issue.

[drcrallen]

Failed java 7 : TaskLifecycleTest.testRealtimeIndexTask:833 null

[drcrallen]

I added some fields in a separate commit that permit backwards compatibility for the known problem classes.

This is intended to make the upgrade process go smoothly.

It is intended that some point in the future the second commit can simply be reverted to remove the backwards compatibility once the upgrade has gone smoothly. So maybe 0.9.1?

[drcrallen]

@guobingkun / @fjy any other comments?

[fjy]

👍

[drcrallen]

Tentatively adding a Release Notes tag here, though I'm not quite certain what to say about it other than "version may require more testing than normal to ensure your configs don't get weird"