Implement rdap_query command

[sharma1210]

This PR implements the rdap_query command as a replacement for whois_query.
https://b.corp.google.com/issues/408247441

This change introduces an enhanced error handling mechanism for the command. It now parses the JSON body of RDAP error responses to present a clean, user-friendly message.

I've added this to improve the user experience for handled failures (like 404s). If you feel this is overly complex or not needed, I'm happy to remove it and revert to simpler error logging.

---

This change is 

[ptkach]

I think this duplicated Chrome pretty-print functionality as sent in chat.

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

[ptkach]

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

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 81 at r1 (raw file):

      JsonElement rdapJson = JsonParser.parseString(rdapResponse);

      System.out.println(formatJsonElement(rdapJson, ""));

Have you considered using GSON for formatting the ouput? We already use it in other parts of the project and it would shave of ~30lines of this file.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 109 at r1 (raw file):

  /** Formats a user-friendly error message from the IOException details. */
  private String formatUserFriendlyError(String path, String errorMessage) {

Same here, I think you can simplify with GSON

[gbrodman]

@gbrodman reviewed 2 of 4 files at r1, all commit messages.
Reviewable status: 2 of 4 files reviewed, 10 unresolved discussions (waiting on @ptkach and @sharma1210)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 1 at r1 (raw file):

// Copyright 2024 The Nomulus Authors. All Rights Reserved.

2025

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 43 at r1 (raw file):

  @Parameter(
      description = "The ordered RDAP path segments that form the path (e.g., 'domain foo.dev').",

Doing the parameters this way forces the caller to know / look up for themselves what the RDAP query options are, and relies on them formatting everything properly. Instead, we should make it easy to get it right (and difficult to get it wrong)

maybe something like

nomulus rdap_query --type=domain foo.dev
nomulus rdap_query --type=domainSearch foo.*
nomulus rdap_query --type=nameserver ns1.googledomains.com

so the type argument would be an enum that we define in this class, and that would define the URL format we'd use (e.g. /rdap/domain/foo.dev for the first one and /rdap/domains?name=foo.*

(we can probably skip super complicated cases like "searching for a domain by nameserver IP", at least for now)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 66 at r1 (raw file):

  @Override
  public void run() {

unnecessary whitespaces

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 72 at r1 (raw file):

    try {
      if (defaultConnection == null) {

can replace this block with one line using Guava Preconditions

checkState(defaultConnection != null, "ServiceConnection was not set by RegistryCli");

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 81 at r1 (raw file):

      JsonElement rdapJson = JsonParser.parseString(rdapResponse);

      System.out.println(formatJsonElement(rdapJson, ""));

use a FluentLogger, not sysout (same elsewhere in the file too)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 83 at r1 (raw file):

      System.out.println(formatJsonElement(rdapJson, ""));

    } catch (IOException e) {

We don't need to catch IOExceptions, just let them fall through.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 124 at r1 (raw file):

  /** Attempts to parse a JSON object from the IOException message. */
  private Optional<JsonObject> parseErrorJson(String errorMessage) {

seconded using GSON for everything, though tbh we should just let the IOException propagate

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 40 at r1 (raw file):

  @Mock private ServiceConnection mockPubapiConnection;

  @Config("useCanary")

this annotation doesn't make sense here, it's used for injection of parameters during normal runtime

[sharma1210]

resolved all the comments

[gbrodman]

Please respond to the comments individually in Reviewable so that they get resolved and go away (and we know that you've seen them), otherwise it can get confusing and Reviewable won't handle the attention set properly

Reviewable status: 2 of 4 files reviewed, 10 unresolved discussions (waiting on @ptkach and @sharma1210)

[gbrodman]

Reviewable status: 2 of 4 files reviewed, 13 unresolved discussions (waiting on @ptkach and @sharma1210)

---

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

  enum RdapQueryType {
    DOMAIN,
    DOMAINSEARCH,

multiple words need to be snake_case https://engdoc.corp.google.com/eng/doc/devguide/java/style/index.md?cl=head#s5.2.4-constant-names

---

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

    ImmutableMap<String, String> queryParams;

    switch (type) {

The conversion from "rdap query type" to "query path and parameter map" is innate to the query types themselves, so those should be defined as methods on the enum class

---

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

        break;
      case NAMESERVER:
        checkArgument(queryTerm != null, "A query term is required for a NAMESERVER lookup.");

we shouldn't need to repeat this checkArgument line every time

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 13 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 1 at r1 (raw file):

Previously, gbrodman wrote…

2025

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 43 at r1 (raw file):

Previously, gbrodman wrote…

Doing the parameters this way forces the caller to know / look up for themselves what the RDAP query options are, and relies on them formatting everything properly. Instead, we should make it easy to get it right (and difficult to get it wrong)

maybe something like

nomulus rdap_query --type=domain foo.dev
nomulus rdap_query --type=domainSearch foo.*
nomulus rdap_query --type=nameserver ns1.googledomains.com

so the type argument would be an enum that we define in this class, and that would define the URL format we'd use (e.g. /rdap/domain/foo.dev for the first one and /rdap/domains?name=foo.*

(we can probably skip super complicated cases like "searching for a domain by nameserver IP", at least for now)

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 66 at r1 (raw file):

Previously, gbrodman wrote…

unnecessary whitespaces

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 72 at r1 (raw file):

Previously, gbrodman wrote…

can replace this block with one line using Guava Preconditions

checkState(defaultConnection != null, "ServiceConnection was not set by RegistryCli");

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 81 at r1 (raw file):

Previously, ptkach (Pavlo Tkach) wrote…

Have you considered using GSON for formatting the ouput? We already use it in other parts of the project and it would shave of ~30lines of this file.

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 81 at r1 (raw file):

Previously, gbrodman wrote…

use a FluentLogger, not sysout (same elsewhere in the file too)

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 83 at r1 (raw file):

Previously, gbrodman wrote…

We don't need to catch IOExceptions, just let them fall through.

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 109 at r1 (raw file):

Previously, ptkach (Pavlo Tkach) wrote…

Same here, I think you can simplify with GSON

Done.

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 124 at r1 (raw file):

Previously, gbrodman wrote…

seconded using GSON for everything, though tbh we should just let the IOException propagate

Done.

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 13 unresolved discussions (waiting on @gbrodman and @ptkach)

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 13 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 40 at r1 (raw file):

Previously, gbrodman wrote…

this annotation doesn't make sense here, it's used for injection of parameters during normal runtime

Done.

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 13 unresolved discussions (waiting on @gbrodman and @ptkach)

---

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

Previously, gbrodman wrote…

multiple words need to be snake_case https://engdoc.corp.google.com/eng/doc/devguide/java/style/index.md?cl=head#s5.2.4-constant-names

Done.

---

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

Previously, gbrodman wrote…

The conversion from "rdap query type" to "query path and parameter map" is innate to the query types themselves, so those should be defined as methods on the enum class

Done.

---

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

Previously, gbrodman wrote…

we shouldn't need to repeat this checkArgument line every time

Done.

[gbrodman]

@gbrodman reviewed all commit messages.
Reviewable status: 2 of 4 files reviewed, 6 unresolved discussions (waiting on @ptkach)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 39 at r4 (raw file):

  private static final FluentLogger logger = FluentLogger.forEnclosingClass();

  /**

this shouldn't be necessary (see comments below) but in any case, it's better to use records for simple data classes

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 60 at r4 (raw file):

  /** Defines the RDAP query types, encapsulating their path logic and parameter requirements. */
  enum RdapQueryType {

possible idea instead: the enum's constructor arguments could be String urlPathFormat and `Function<String, ImmutableMap<String, String>> queryParametersFunction)

so like

DOMAIN("/rdap/domain/%s", s -> ImmutableMap.of())
DOMAIN_SEARCH("/rdap/domains", s -> ImmutableMap.of("name", s))

then

String getQueryPath(String name) {
  return String.format(urlPathFormat, name);
}

ImmutableMap<String, String> getQueryParameters(String name) {
  return queryParametersFunction.apply(name);
}

maybe using a constructor overload so we only have to write the s -> ImmutableMap.of() bit once

then we don't need a separate RequestData class

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 130 at r4 (raw file):

        pubapiConnection.sendGetRequest(requestData.path, requestData.queryParams);

    JsonElement rdapJson = JsonParser.parseString(rdapResponse);

we can still simplify this with gson, can create a single static Gson object that pretty-prints

[gbrodman]

please respond to the comments in Reviewable, as it makes it clear that you saw them, can prompt discussion, and

@gbrodman reviewed 1 of 2 files at r5, all commit messages.
Reviewable status: 3 of 4 files reviewed, 10 unresolved discussions (waiting on @ptkach and @sharma1210)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 50 at r5 (raw file):

    ENTITY("/rdap/entity/%s"),
    ENTITY_SEARCH("/rdap/entities", queryTerm -> ImmutableMap.of("fn", queryTerm)),
    HELP("/rdap/help", false);

we probably don't need the help command to be honest, so we don't need this boolean at all

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 56 at r5 (raw file):

    @SuppressWarnings("ImmutableEnumChecker")
    private final Function<String, ImmutableMap<String, String>> queryParametersFunction;

or maybe we just have an Optional<String> of the "queryParameterName" to which we map the provided query term. Which looks better to you in implementation?

the main thing is that we should only need two parameters (the path, and something to make the parameters) not 3

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 92 at r5 (raw file):

    String getQueryPath(@Nullable String queryTerm) {
      return getQueryParameters(queryTerm).isEmpty()

don't need this check, can just call String.format

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 53 at r5 (raw file):

    command.setConnection(mockDefaultConnection);
    command.useCanary = false;
    logger.addHandler(logHandler);

Can we use the helper methods in CommandTestCase to test the output? We should be able to. If not, maybe we just go back out to the System.out.println calls to allow us to use the built-in output reader testing materials.

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 55 at r5 (raw file):

    logger.addHandler(logHandler);

    when(mockDefaultConnection.withService(eq(Service.PUBAPI), eq(false)))

fwiw i don't think you need the eq() calls, since we're passing in actual objects for both parameters?

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 10 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 60 at r4 (raw file):

Previously, gbrodman wrote…

possible idea instead: the enum's constructor arguments could be String urlPathFormat and `Function<String, ImmutableMap<String, String>> queryParametersFunction)

so like

DOMAIN("/rdap/domain/%s", s -> ImmutableMap.of())
DOMAIN_SEARCH("/rdap/domains", s -> ImmutableMap.of("name", s))

then

String getQueryPath(String name) {
  return String.format(urlPathFormat, name);
}

ImmutableMap<String, String> getQueryParameters(String name) {
  return queryParametersFunction.apply(name);
}

maybe using a constructor overload so we only have to write the s -> ImmutableMap.of() bit once

then we don't need a separate RequestData class

done

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 50 at r5 (raw file):

Previously, gbrodman wrote…

we probably don't need the help command to be honest, so we don't need this boolean at all

it will definitelly inprove our code if don't need it, but just asking for more clarity is it not needed to explain the format of the command or any confusions

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 56 at r5 (raw file):

Previously, gbrodman wrote…

or maybe we just have an Optional<String> of the "queryParameterName" to which we map the provided query term. Which looks better to you in implementation?

the main thing is that we should only need two parameters (the path, and something to make the parameters) not 3

done

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 92 at r5 (raw file):

Previously, gbrodman wrote…

don't need this check, can just call String.format

done

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 10 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 130 at r4 (raw file):

Previously, gbrodman wrote…

we can still simplify this with gson, can create a single static Gson object that pretty-prints

done

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 10 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 55 at r5 (raw file):

Previously, gbrodman wrote…

fwiw i don't think you need the eq() calls, since we're passing in actual objects for both parameters?

done

[sharma1210]

Reviewable status: 2 of 4 files reviewed, 10 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 53 at r5 (raw file):

Previously, gbrodman wrote…

Can we use the helper methods in CommandTestCase to test the output? We should be able to. If not, maybe we just go back out to the System.out.println calls to allow us to use the built-in output reader testing materials.

Done.

[gbrodman]

@gbrodman reviewed all commit messages.
Reviewable status: 2 of 4 files reviewed, 5 unresolved discussions (waiting on @ptkach)

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 50 at r5 (raw file):

Previously, sharma1210 (sharma1210) wrote…

it will definitelly inprove our code if don't need it, but just asking for more clarity is it not needed to explain the format of the command or any confusions

i'm just saying that we don't need to be able to query for the RDAP help / terms of service, because 1. they're not particularly useful 2. we can just go look at them whenever

---

core/src/main/java/google/registry/tools/RdapQueryCommand.java line 60 at r6 (raw file):

    String getQueryPath(String queryTerm) {
      return searchParamKey.isPresent() ? pathFormat : String.format(pathFormat, queryTerm);

again, you can just call String.format. It's a no-op if there is no %s in the string.

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 64 at r6 (raw file):

    verify(mockPubapiConnection).sendGetRequest(path, ImmutableMap.of());

    assertInStdout("{\n  \"ldhName\": \"example.dev\"\n}");

I think these tests may be slightly overcomplicating things, or doing things that we don't need to do. We're returning mock outputs then verifying that those mock outputs are in stdout, and I don't think we need to test that we print things correctly to stdout. Instead, we should focus more on verifying that the right request was sent to the ServiceConnection object.

check out ListObjectsCommandTestCase for some similar-type tests -- these send requests to the backend so they're a bit different, but basically there we're just verifying that the command "sends" the right request and parameters to the server.

---

core/src/test/java/google/registry/tools/RdapQueryCommandTest.java line 77 at r6 (raw file):

    verify(mockPubapiConnection).sendGetRequest(path, query);

    assertInStdout(

a multiline string would likely be better here, but it might be moot (see other comment)