Add activeOrDeletedSince parameter to RefreshDnsForAllDomainsAction

[jicelhay]

This parameter will be used to enqueue refresh DNS tasks for domains with deletion time >= of its value. This to fix state of deleted domains that have failed calls in the past and are not being picked up by the system.

See b/364282239.

---

This change is 

[CydeWeys]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jicelhay)

---

core/src/main/java/google/registry/tools/server/ToolsServerModule.java line 86 at r1 (raw file):

  @Parameter("deletionTime")
  static Optional<DateTime> provideDeletionTime(HttpServletRequest req) {
    DateTimeFormatter formatter = DateTimeFormat.forPattern("MM-dd-yyyy");

We use ISO 8601-formatted date strings globally throughout the codebase, definitely never MM-dd-yyyy. Just use the same mechanism we use for other DateTime querystring paramters: https://cs.opensource.google/nomulus/nomulus/+/master:core/src/main/java/google/registry/request/RequestParameters.java;l=273-290;drc=49d2e34e12eed5ff52671a8dd747fb9c64522405 (and see usages of that method)

[CydeWeys]

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @jicelhay)

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 55 at r1 (raw file):

 * recommended to be somewhere between 200 and 500. The default value is 250.
 *
 * <p>If {@code deletionTime} is passed in the request, this action will enqueue DNS publish tasks

&deletionTime= as a parameter name doesn't quite get at the intent of what this new parameter does (and a naive reading of it would assume that it's only processing deleted domains, which is the opposite of what it's actually doing). I might suggest instead using &activeSince=, as that most properly describes the intent of using this parameter, as in, "I want to refresh the DNS of all domains that were active since January 2023."

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 57 at r1 (raw file):

 * <p>If {@code deletionTime} is passed in the request, this action will enqueue DNS publish tasks
 * on all domains with a deletion time equal or greater than the value provided and there's no
 * guaranteed that they will be active. {@code deletionTime} will be set at 0:00:00 UTC zone for the

s/and there's no guaranteed that they will be active/including domains that have since been deleted/

Code quote:

 * on all domains with a deletion time equal or greater than the value provided and there's no
 * guaranteed that they will be active. {@code deletionTime} will be set at 0:00:00 UTC zone for the

[jicelhay]

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @CydeWeys)

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 55 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

&deletionTime= as a parameter name doesn't quite get at the intent of what this new parameter does (and a naive reading of it would assume that it's only processing deleted domains, which is the opposite of what it's actually doing). I might suggest instead using &activeSince=, as that most properly describes the intent of using this parameter, as in, "I want to refresh the DNS of all domains that were active since January 2023."

Fair, but I also think that having a param named activeSince, with the caveat that "there's no guarantee that they will be active" is equally if not more confusing for readers.

What do you think of activeAt? I think that conveys that the domain was active at param value, but might have change status since.

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 57 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

s/and there's no guaranteed that they will be active/including domains that have since been deleted/

Done.

---

core/src/main/java/google/registry/tools/server/ToolsServerModule.java line 86 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

We use ISO 8601-formatted date strings globally throughout the codebase, definitely never MM-dd-yyyy. Just use the same mechanism we use for other DateTime querystring paramters: https://cs.opensource.google/nomulus/nomulus/+/master:core/src/main/java/google/registry/request/RequestParameters.java;l=273-290;drc=49d2e34e12eed5ff52671a8dd747fb9c64522405 (and see usages of that method)

Done.

[CydeWeys]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jicelhay)

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 55 at r1 (raw file):

Previously, jicelhay (Juan Celhay) wrote…

Fair, but I also think that having a param named activeSince, with the caveat that "there's no guarantee that they will be active" is equally if not more confusing for readers.

What do you think of activeAt? I think that conveys that the domain was active at param value, but might have change status since.

So I see where you're coming from, but activeAt is also not quite right, as it seems to imply that a domain was active at a given date in order to be refreshed, but this action will be refreshing domains that were created after that time (and thus were not active then). What we really want is wording that captures the meaning of "active at any time since X". Or alternatively, notDeletedBefore? That's accurate, right? A little strange to phrase the parameter negatively, but if that's the most accurate wording possible, then ...

[CydeWeys]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jicelhay)

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 55 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

So I see where you're coming from, but activeAt is also not quite right, as it seems to imply that a domain was active at a given date in order to be refreshed, but this action will be refreshing domains that were created after that time (and thus were not active then). What we really want is wording that captures the meaning of "active at any time since X". Or alternatively, notDeletedBefore? That's accurate, right? A little strange to phrase the parameter negatively, but if that's the most accurate wording possible, then ...

activeOrDeletedSince is accurate too, but wordier.

[jicelhay]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @CydeWeys)

---

core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java line 55 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

activeOrDeletedSince is accurate too, but wordier.

I prefer activeOrDeletedSince. notDeletedBefore might be confusing as well because readers might assume that it's deleted after that, which we know is not true.

[CydeWeys]

Just need to update the commit description now with the new param name.

Reviewable status: 0 of 4 files reviewed, all discussions resolved

[jicelhay]

Done.

Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @CydeWeys)

[CydeWeys]

Reviewable status: 0 of 4 files reviewed, all discussions resolved

[CydeWeys]

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jicelhay)