Add Java classes for console history objects

[gbrodman]

This also creates base classes for the objects contained within the history classes, e.g. RegistrarBase. This is the same way that objects stored in the HistoryEntry subclasses have base classes, e.g. DomainBase.

---

This change is 

[jianglai]

Reviewed 79 of 81 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @gbrodman)

---

core/src/main/java/google/registry/model/console/ConsoleUpdateHistory.java line 61 at r2 (raw file):

  /** The user that performed the modification. */
  @JoinColumn(name = "historyActingUser", referencedColumnName = "emailAddress")

nullable = false?

---

core/src/main/java/google/registry/model/console/ConsoleUpdateHistory.java line 78 at r2 (raw file):

  /** The time at which the modification was mode. */
  @Column(nullable = false, name = "historyDateTime")

It might be a bit late (because the schema change has gone in), but I think we should probably keep consistency across history objects. In EPP history resources we use historyModificationTime I believe. I'll leave it up to you, depending on how much trouble you are willing to go through (at least there's no data in the table yet) and how soon you want to commit these changes. I actually think it is OK to change schema and the code at the same time, as long as you are not actively using the table. But our presubmits might prevent you from doing that.

---

core/src/main/java/google/registry/model/console/ConsoleUpdateHistory.java line 88 at r2 (raw file):

  public long getRevisionId() {
    // For some reason, Hibernate throws NPE during some initialization phases if we don't deal with
    // the null case. Setting the id to 0L when it is null should be fine because 0L for primitive

Is it still true? Or did you just copy-and-pasted this part?

---

core/src/main/java/google/registry/ui/server/RegistrarFormFields.java line 93 at r2 (raw file):

          .build();

  public static final FormField<String, Registrar.State> STATE_FIELD =

These removals seem unrelated to the PR?

---

core/src/test/java/google/registry/testing/DatabaseHelper.java line 1018 at r2 (raw file):

  }

  /** Creates and persists an admin {@link User} with the given email address. */

if it does not already exist.

---

core/src/test/java/google/registry/testing/DatabaseHelper.java line 1031 at r2 (raw file):

            .build();
    tm().transact(() -> tm().put(user));
    return UserDao.loadUser(emailAddress).get();

Any reason to load it back from the database? Are there any (significant) fields that only get set when the entity is persisted?

[gbrodman]

Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @Github-advanced-security[bot] and @jianglai)

---

core/src/main/java/google/registry/model/console/ConsoleUpdateHistory.java line 61 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

nullable = false?

Done.

---

core/src/main/java/google/registry/model/console/ConsoleUpdateHistory.java line 78 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

It might be a bit late (because the schema change has gone in), but I think we should probably keep consistency across history objects. In EPP history resources we use historyModificationTime I believe. I'll leave it up to you, depending on how much trouble you are willing to go through (at least there's no data in the table yet) and how soon you want to commit these changes. I actually think it is OK to change schema and the code at the same time, as long as you are not actively using the table. But our presubmits might prevent you from doing that.

I think the presubmits would prevent that, though it's not like we're in a huge rush here. The name "modification time" kind of felt odd to me before, but I suppose it's referring to the time that the resource in question was modified? I would be amenable to changing it here, I think.

---

core/src/main/java/google/registry/model/console/ConsoleUpdateHistory.java line 88 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

Is it still true? Or did you just copy-and-pasted this part?

I copied and pasted this from HistoryEntry, since this is kind of modeled on that class.

it seems like it passes the tests without it though soooooooooo yeah we can remove it here

---

core/src/main/java/google/registry/ui/server/RegistrarFormFields.java line 93 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

These removals seem unrelated to the PR?

They're unused, and we were already updating the enums in this class anyway.

---

core/src/test/java/google/registry/model/console/RegistrarPocUpdateHistoryTest.java line 39 at r1 (raw file):

Previously, github-advanced-security[bot] wrote…

Unread local variable

Variable 'ImmutableList pocs' is never read.

Show more details

Done.

---

core/src/test/java/google/registry/testing/DatabaseHelper.java line 1018 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

if it does not already exist.

Done.

---

core/src/test/java/google/registry/testing/DatabaseHelper.java line 1031 at r2 (raw file):

Previously, jianglai (Lai Jiang) wrote…

Any reason to load it back from the database? Are there any (significant) fields that only get set when the entity is persisted?

the ID, unfortunately

[jianglai]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @gbrodman and @Github-advanced-security[bot])

---

core/src/test/java/google/registry/testing/DatabaseHelper.java line 1031 at r2 (raw file):

Previously, gbrodman wrote…

the ID, unfortunately

Ah, of course...

[gbrodman]

(this will still fail because the other change has not been deployed)

[gbrodman]

@jianglai just checking, are you still good with this? It's been a while since it was in review (due to me going on vacation) so I just wanted to double-check

[jianglai]

@jianglai just checking, are you still good with this? It's been a while since it was in review (due to me going on vacation) so I just wanted to double-check

Yes, go ahead.