ebean icon indicating copy to clipboard operation
ebean copied to clipboard

Proposal: Always use JSON String format for Date/DateTime. Remove formatting as millis and nanos.

Open rbygrave opened this issue 2 years ago • 7 comments

Proposal

Remove the support for JSON formatting Date as millis and DateTime as millis or nanos. This means that all Dates and DateTime types would be formatted in JSON string format.

I believe that this feature doesn't make good sense in 2021.

Is there anyone relying on this feature and is requesting that it remain? If so can you outline why you think we should keep this feature.

Thanks.

Reference:

https://github.com/ebean-orm/ebean/blob/master/ebean-api/src/main/java/io/ebean/config/JsonConfig.java#L16 https://github.com/ebean-orm/ebean/blob/master/ebean-api/src/main/java/io/ebean/config/JsonConfig.java#L37

rbygrave avatar Dec 15 '21 00:12 rbygrave

We would need some kind of backward comptibility. So that old data (written in millis in our case) is still readable.

This may also affect other people when #2457 is dropped, as the joda string format could be slightly different from the java8 time api

rPraml avatar Dec 15 '21 13:12 rPraml

old data (written in millis in our case) is still readable.

Hmmm, that might make it near impossible to actually remove the millis/nanos format option.

joda string format could be slightly different

Need to test this.

rbygrave avatar Dec 16 '21 21:12 rbygrave

Why would you want to remove such options? Does it hurt somehow? If ISO format isn't the default, then maybe just change the default.

jnehlmeier avatar Dec 17 '21 09:12 jnehlmeier

Does it hurt somehow?

It adds to the complexity of ScalarType, well the complexity of all the date/datetime types.

If ISO format isn't the default

ISO8601 is the default

rbygrave avatar Dec 17 '21 10:12 rbygrave

Hmmm, that might make it near impossible to actually remove the millis/nanos format option.

Should not be too difficult, I think we have to probe different formats. See: https://github.com/FOCONIS/ebean/commit/68a82a049ac85e1619bd3b4da1173c2ac288b976#diff-f281574378f17447285b13aed7a781ddd33559d8a2a7f5370adc40ea29cbc02aR66

rPraml avatar Dec 17 '21 13:12 rPraml

I think we have to probe different formats. See

Yes I see, good thinking. That would make it possible.

rbygrave avatar Dec 19 '21 19:12 rbygrave

remove such options?

So just some background / "rob philosophy" on the general topic of removing features.

Ebean should not do more than it needs to do.

With Ebean we have done 2 rounds of "feature removal" over the course of what is now just over 15 years as an open source project. That is, at version 4.x and 10.x we looked relatively harshly at our API and features and removed what seemed reasonable at the time. For example, Ebean in 3.x had an abstraction over LDAP and that got removed in 4.x. In general I believe that feature removal has helped Ebean long term.

In terms of size of jars and size of code base, Ebean is a lot smaller than the ORM's we compare ourselves to. Part of that is down to making arguably better design choices (like the query language), part down to mandating enhancement (no dynamic proxies) and part down to trying to keep the feature set as close to minimal as we can and doing those 2 rounds of feature removal.

If I recall correctly there was a couple of cases where we got feedback that lead to features being re-instated. One was findIterate() and the other was json visitors. So we haven't been perfect in doing this and I think some people will disagree with the philosophy and approach. This approach works better when people actively participate.

This sort of change isn't ideal. Ideally Ebean is "feature set done", "stable", "boring", in maintenance with bug fixes and general up keep the only changes going on. The argument is that those 2 "feature removal" efforts were not ideal but were good for Ebean long term and now we are really really close to "feature set done".

The state today

In terms of feature set, Ebean is very close to "feature set done". This means from now on we don't expect much at all in terms of feature removal or addition from now on.

We still need to deal with databases that change/add features (like MariaDB sequences), we still need some improvement in supporting things like open tracing etc, in moving to java 11 we might improve the modular area (pull each platform into separate modules, ebean-platform-postgres, ebean-platform-sqlserver ...). So there is still a lot of work to do etc but there shouldn't be much at all in terms of feature addition or removal.

remove such options?

So coming back to the question of removing the feature. Ideally people participate in the process and really provide feedback in terms of whether this feature is actually used. My gut says we probably shouldn't use the json millis format option, my gut says 99.9% of people will not being using it (as it isn't the default).

So should we keep this feature for the 0.1%? Who uses it and why? Ideally we get a good feel for this in the form of feedback.

If the answer is, no we don't use this feature ... then that's a strong indication that we shouldn't probably have the feature and try to keep Ebean a little bit simpler, leaner, cleaner, lighter.

rbygrave avatar Dec 19 '21 21:12 rbygrave

Ok, so quite a bit of refactoring and tidy up work has just gone into the ScalarType area and we have moved the ScalarTypeBase, ScalarTypeBaseDate, ScalarTypeBaseDateTime into ebean-core-type module. We also pulled out the CsvReader into a separate module and more specifically removed it's need for a few methods on ScalarType.

So with that all done and looking at the result I'm happy enough to leave the JSON format options as they currently are.

That means, I think we close off this issue as "will not change".

Now that said, I'm confident that I myself would always desire ISO string formatting for dates and dateTimes in apps when I get the choice so we are leaving in options that I'd personally never choose (which isn't a great sign generally) but at this point in time I'm happy enough that we are not paying too much complexity wise for this and happy to leave it in at this stage.

Ok with that @rPraml @jnehlmeier ? We can look to revisit in some months or a year or so later.

rbygrave avatar Aug 26 '22 05:08 rbygrave

Closing for now. Will reconsider this down the track sometime.

rbygrave avatar Aug 29 '22 23:08 rbygrave