Remove java.util.Date

[jianglai]

There is one remaining instance in JpaTransactionManagerImpl that cannot
be removed because DetachingTypedQuery is implementing TypedQuery, which has
a method that expectred java.util.Date.

---

This change is 

[jianglai]

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jianglai)

[weiminyu]

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jianglai)

[CydeWeys]

What is the Java.date package? That doesn't seem correct to me.

[CydeWeys]

This is a functionality change? Are you sure this is even correct?

What's the motivation for removing Date everywhere anyway? Sometimes you do legitimately need to process just a date, not a date and an attached time.

[CydeWeys]

This seems like a bad change to me? Why are we going to make people specify an entire date time on the command line when the only thing that should be passed in is a date?

[CydeWeys]

This is literally still using a java.util.Date then, only it's purposely being packaged into a DateTime, and then stripped out of it, for no reason!

[jianglai]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @CydeWeys)

---

config/presubmits.py line 192 at r2 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

What is the Java.date package? That doesn't seem correct to me.

My bad. Should be java.time.

---

core/src/main/java/google/registry/flows/certs/CertificateChecker.java line 161 at r2 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

This is a functionality change? Are you sure this is even correct?

What's the motivation for removing Date everywhere anyway? Sometimes you do legitimately need to process just a date, not a date and an attached time.

java.util.Date is not just a date. It's an instant in time with millisecond precision. There are many problems with it(no timezone support, no calendar support, etc) and most of its methods are deprecated for decades (since JDK 1.1). That's the whole reason why people prefer Joda Time instead of Java.util.Date.

---

core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java line 63 at r2 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

This seems like a bad change to me? Why are we going to make people specify an entire date time on the command line when the only thing that should be passed in is a date?

Even with Date people are going to have to specify the time (see the other comment on Data being an instant in time). I don't know how jcommander handlers Date conversions, it may work if you don't specify one and it gives you some default time, but without timezone support that time is also ambiguous. We have been using DateTime in all other commands.

In general java.util.Date has such bad APIs that everyone should run from it unless they absolutely have to (like when their dependencies still expect it). Even then one should defer the conversion to Date until the last minute, preferably manipulating Joda Time or java.time until they call toDate() to convert it and pass it to the dependencies.

---

networking/src/test/java/google/registry/networking/handler/SslInitializerTestUtils.java line 85 at r2 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

This is literally still using a java.util.Date then, only it's purposely being packaged into a DateTime, and then stripped out of it, for no reason!

We have no control over how our dependencies' interfaces are defined, but we can avoid directly interacting with the deprecated java.util.date API as much as possible.

[jianglai]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @CydeWeys)

[CydeWeys]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @jianglai)

---

core/src/main/java/google/registry/flows/certs/CertificateChecker.java line 161 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

java.util.Date is not just a date. It's an instant in time with millisecond precision. There are many problems with it(no timezone support, no calendar support, etc) and most of its methods are deprecated for decades (since JDK 1.1). That's the whole reason why people prefer Joda Time instead of Java.util.Date.

How about https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/time/LocalDate.html then? That looks to do what we need it to do here, but without that baggage.

[jianglai]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @CydeWeys)

---

core/src/main/java/google/registry/flows/certs/CertificateChecker.java line 161 at r2 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

How about https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/time/LocalDate.html then? That looks to do what we need it to do here, but without that baggage.

In this case, we definitely want both a date and a time (as it pertains to checking the expiration date of certificates or CRLs).

In general, the recommendation is to use java.time for greenfield projects and joda.time for existing project. We have a mixture of both at the moment, though I think it might be a good idea to pick a lane and stick to it to avoid unnecessary conversions. Joda actually recommends actively migrating to java.time, but we have a lot of joda.time and the migration might not be worth the effort.

[jianglai]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @CydeWeys)

Addressed the comment.