diff --git a/java/google/registry/flows/FlowReporter.java b/java/google/registry/flows/FlowReporter.java index b978ec8f4..0f776cc94 100644 --- a/java/google/registry/flows/FlowReporter.java +++ b/java/google/registry/flows/FlowReporter.java @@ -16,12 +16,18 @@ package google.registry.flows; import static com.google.common.io.BaseEncoding.base64; import static google.registry.xml.XmlTransformer.prettyPrint; +import static java.util.Collections.EMPTY_LIST; +import com.google.common.base.Ascii; +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import google.registry.flows.FlowModule.ClientId; import google.registry.flows.FlowModule.InputXml; import google.registry.flows.annotations.ReportingSpec; import google.registry.model.eppcommon.Trid; +import google.registry.model.eppinput.EppInput; import google.registry.util.FormattingLogger; import javax.inject.Inject; import org.json.simple.JSONValue; @@ -48,6 +54,7 @@ public class FlowReporter { @Inject Trid trid; @Inject @ClientId String clientId; @Inject @InputXml byte[] inputXmlBytes; + @Inject EppInput eppInput; @Inject Class flowClass; @Inject FlowReporter() {} @@ -64,13 +71,57 @@ public class FlowReporter { // Explicitly log flow metadata separately from the EPP XML itself so that it stays compact // enough to be sure to fit in a single log entry (the XML part in rare cases could be long // enough to overflow into multiple log entries, breaking routine parsing of the JSON format). + String resourceType = eppInput.getResourceType().or(""); + boolean isDomain = "domain".equals(resourceType); + String singleTargetId = eppInput.getSingleTargetId().or(""); + ImmutableList targetIds = eppInput.getTargetIds(); logger.infofmt( "%s: %s", METADATA_LOG_SIGNATURE, - JSONValue.toJSONString(ImmutableMap.of( - "trid", trid.getServerTransactionId(), - "clientId", clientId, - "icannActivityReportField", extractActivityReportField(flowClass)))); + JSONValue.toJSONString(new ImmutableMap.Builder() + .put("serverTrid", trid.getServerTransactionId()) + .put("clientId", clientId) + .put("commandType", eppInput.getCommandType()) + .put("resourceType", resourceType) + .put("flowClassName", flowClass.getSimpleName()) + .put("targetId", singleTargetId) + .put("targetIds", targetIds) + .put("tld", isDomain ? extractTld(singleTargetId).or("") : "") + .put("tlds", isDomain ? extractTlds(targetIds).asList() : EMPTY_LIST) + .put("icannActivityReportField", extractActivityReportField(flowClass)) + .build())); + } + + /** + * Returns the guessed TLD of the given domain name, assuming a second-level domain name, or + * absent if no TLD could be detected. + * + *

This method is quick and dirty and doesn't attempt to validate the domain name in any way; + * it just takes anything after the first period to be the TLD and converts ASCII to lowercase. + * We want quick and dirty here because this will be called on not-yet-validated EPP XML where + * just about anything could be supplied, and there's no reason to validate twice when this just + * needs to be roughly correct. + */ + private static final Optional extractTld(String domainName) { + int index = domainName.indexOf('.'); + return index == -1 + ? Optional.absent() + : Optional.of(Ascii.toLowerCase(domainName.substring(index + 1))); + } + + /** + * Returns the set of unique results of {@link #extractTld} applied to each given domain name, + * excluding any absent results (i.e. cases where no TLD was detected). + */ + private static final ImmutableSet extractTlds(Iterable domainNames) { + ImmutableSet.Builder set = new ImmutableSet.Builder<>(); + for (String domainName : domainNames) { + Optional extractedTld = extractTld(domainName); + if (extractedTld.isPresent()) { + set.add(extractedTld.get()); + } + } + return set.build(); } /** diff --git a/javatests/google/registry/flows/FlowReporterTest.java b/javatests/google/registry/flows/FlowReporterTest.java index 890945b19..6b1a56ff4 100644 --- a/javatests/google/registry/flows/FlowReporterTest.java +++ b/javatests/google/registry/flows/FlowReporterTest.java @@ -19,13 +19,17 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions; import static java.nio.charset.StandardCharsets.UTF_8; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import com.google.common.base.Optional; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.testing.TestLogHandler; import google.registry.flows.annotations.ReportingSpec; import google.registry.model.eppcommon.Trid; +import google.registry.model.eppinput.EppInput; import google.registry.model.eppoutput.EppOutput.ResponseOrGreeting; import google.registry.model.eppoutput.EppResponse; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; @@ -68,6 +72,11 @@ public class FlowReporterTest extends ShardableTestCase { flowReporter.clientId = "TheRegistrar"; flowReporter.inputXmlBytes = "".getBytes(UTF_8); flowReporter.flowClass = TestCommandFlow.class; + flowReporter.eppInput = mock(EppInput.class); + when(flowReporter.eppInput.getCommandType()).thenReturn("info"); + when(flowReporter.eppInput.getResourceType()).thenReturn(Optional.of("domain")); + when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("target.foo")); + when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("target.foo")); } @Test @@ -96,8 +105,15 @@ public class FlowReporterTest extends ShardableTestCase { flowReporter.recordToLogs(); assertThat(parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "))) .containsExactly( - "trid", "server-456", + "serverTrid", "server-456", "clientId", "TheRegistrar", + "commandType", "info", + "resourceType", "domain", + "flowClassName", "TestCommandFlow", + "targetId", "target.foo", + "targetIds", ImmutableList.of("target.foo"), + "tld", "foo", + "tlds", ImmutableList.of("foo"), "icannActivityReportField", ""); } @@ -105,22 +121,109 @@ public class FlowReporterTest extends ShardableTestCase { public void testRecordToLogs_metadata_withReportingSpec() throws Exception { flowReporter.flowClass = TestReportingSpecCommandFlow.class; flowReporter.recordToLogs(); - assertThat(parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "))) - .containsExactly( - "trid", "server-456", - "clientId", "TheRegistrar", - "icannActivityReportField", "srs-cont-check"); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("flowClassName", "TestReportingSpecCommandFlow"); + assertThat(json).containsEntry("icannActivityReportField", "srs-cont-check"); } @Test public void testRecordToLogs_metadata_noClientId() throws Exception { flowReporter.clientId = ""; flowReporter.recordToLogs(); - assertThat(parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: "))) - .containsExactly( - "trid", "server-456", - "clientId", "", - "icannActivityReportField", ""); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("clientId", ""); + } + + @Test + public void testRecordToLogs_metadata_notResourceFlow_noResourceTypeOrTld() throws Exception { + when(flowReporter.eppInput.getResourceType()).thenReturn(Optional.absent()); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("resourceType", ""); + assertThat(json).containsEntry("tld", ""); + assertThat(json).containsEntry("tlds", ImmutableList.of()); + } + + + @Test + public void testRecordToLogs_metadata_notDomainFlow_noTld() throws Exception { + when(flowReporter.eppInput.getResourceType()).thenReturn(Optional.of("contact")); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("resourceType", "contact"); + assertThat(json).containsEntry("tld", ""); + assertThat(json).containsEntry("tlds", ImmutableList.of()); + } + + @Test + public void testRecordToLogs_metadata_multipartDomainName_multipartTld() throws Exception { + when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("target.co.uk")); + when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("target.co.uk")); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("targetId", "target.co.uk"); + assertThat(json).containsEntry("targetIds", ImmutableList.of("target.co.uk")); + assertThat(json).containsEntry("tld", "co.uk"); + assertThat(json).containsEntry("tlds", ImmutableList.of("co.uk")); + } + + @Test + public void testRecordToLogs_metadata_multipleTargetIds_uniqueTldSet() throws Exception { + when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.absent()); + when(flowReporter.eppInput.getTargetIds()) + .thenReturn(ImmutableList.of("target.co.uk", "foo.uk", "bar.uk", "baz.com")); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("targetId", ""); + assertThat(json).containsEntry( + "targetIds", ImmutableList.of("target.co.uk", "foo.uk", "bar.uk", "baz.com")); + assertThat(json).containsEntry("tld", ""); + assertThat(json).containsEntry("tlds", ImmutableList.of("co.uk", "uk", "com")); + } + + @Test + public void testRecordToLogs_metadata_uppercaseDomainName_lowercaseTld() throws Exception { + when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("TARGET.FOO")); + when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("TARGET.FOO")); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("targetId", "TARGET.FOO"); + assertThat(json).containsEntry("targetIds", ImmutableList.of("TARGET.FOO")); + assertThat(json).containsEntry("tld", "foo"); + assertThat(json).containsEntry("tlds", ImmutableList.of("foo")); + } + + @Test + public void testRecordToLogs_metadata_invalidDomainName_stillGuessesTld() throws Exception { + when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("")); + when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("")); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("targetId", ""); + assertThat(json).containsEntry("targetIds", ImmutableList.of("")); + assertThat(json).containsEntry("tld", "com>"); + assertThat(json).containsEntry("tlds", ImmutableList.of("com>")); + } + + @Test + public void testRecordToLogs_metadata_domainWithoutPeriod_noTld() throws Exception { + when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("target,foo")); + when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("target,foo")); + flowReporter.recordToLogs(); + Map json = + parseJsonMap(findLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); + assertThat(json).containsEntry("targetId", "target,foo"); + assertThat(json).containsEntry("targetIds", ImmutableList.of("target,foo")); + assertThat(json).containsEntry("tld", ""); + assertThat(json).containsEntry("tlds", ImmutableList.of()); } @SuppressWarnings("unchecked") diff --git a/javatests/google/registry/flows/ResourceFlowTestCase.java b/javatests/google/registry/flows/ResourceFlowTestCase.java index 1559d1d1d..ca0790bca 100644 --- a/javatests/google/registry/flows/ResourceFlowTestCase.java +++ b/javatests/google/registry/flows/ResourceFlowTestCase.java @@ -45,6 +45,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import org.joda.time.DateTime; import org.joda.time.Duration; +import org.json.simple.JSONValue; import org.junit.Before; import org.junit.Test; @@ -173,10 +174,25 @@ public abstract class ResourceFlowTestCase