rharter / auto-value-gson

AutoValue Extension to add Gson De/Serializer support
Apache License 2.0
603 stars 103 forks source link

Optionally initialize known collection types to empty, instead of null #217

Closed cushon closed 4 years ago

cushon commented 5 years ago

Context:

cushon commented 5 years ago

Would you be willing to consider restoring support for this feature? Its removal is preventing us from updating to head. I understand from #158 this may not have been a complete solution, but removing it without a replacement feels like a step backwards.

ZacSweers commented 5 years ago

136 is the ideal replacement, but is blocked by https://github.com/google/auto/pull/501 and something that Google does have control over. Not sure how connected you are with that project, but what would you think of revisiting that? I think if that API were finally reviewed, we'd be happy to do a new release with support for it and support property normalization like this indirectly

rharter commented 5 years ago

Sorry for the delay on this, I've been on parental leave so haven't been doing much in the projects.

IIRC the problem with #158 was largely in the default values, as opposed to the empty lists. Would it be helpful to have empty list support without default values? It seems like that would be a relatively benign change, which I'd be okay with, but it's also largely a modelling issue (e.g. if the property should be an empty list, make it an empty list in json).

cushon commented 5 years ago

136 is the ideal replacement, but is blocked by google/auto#501 and something that Google does have control over. Not sure how connected you are with that project, but what would you think of revisiting that?

It sounds like there's still interest in that PR, but the change needs to go through our API review proceses and I don't have an ETA for that yet.

Would it be helpful to have empty list support without default values?

It would be, because we started depending on this handling of empty lists after 15564443795bdf3f0351db41d6de770cc2ceba1d was added, and the removal of that support is making it hard to upgrade to the latest version of auto-value-gson.

We investigated modifying the affected uses of auto-value-gson to use explicit empty lists instead of relying on this support, but it would be an invasive cleanup and it isn't clear it's an improvement for those use-cases.

ZacSweers commented 4 years ago

Going to close this now that #136 is available. The solution for this would be to create a builder factory that defaults collections to empty. Note that if the collection is null in JSON, it would be overwritten to null in the builder!