Merge pull request #97 from gbrodman/spec11DNS

Only inspect DNS-published domains in Spec11

core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java

@@ -17,16 +17,20 @@ package google.registry.reporting.spec11;
 import static com.google.common.base.Throwables.getRootCause;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.io.Resources.getResource;
+import static google.registry.model.ofy.ObjectifyService.ofy;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.net.MediaType;
 import com.google.template.soy.SoyFileSet;
 import com.google.template.soy.parseinfo.SoyTemplateInfo;
 import com.google.template.soy.tofu.SoyTofu;
 import com.google.template.soy.tofu.SoyTofu.Renderer;
+import google.registry.beam.spec11.ThreatMatch;
 import google.registry.config.RegistryConfig.Config;
+import google.registry.model.domain.DomainBase;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo;
@@ -34,7 +38,6 @@ import google.registry.util.EmailMessage;
 import google.registry.util.SendEmailService;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import javax.inject.Inject;
 import javax.mail.MessagingException;
 import javax.mail.internet.InternetAddress;
@@ -84,18 +87,21 @@ public class Spec11EmailUtils {
       LocalDate date,
       SoyTemplateInfo soyTemplateInfo,
       String subject,
-      Set<RegistrarThreatMatches> registrarThreatMatchesSet) {
+      ImmutableSet<RegistrarThreatMatches> registrarThreatMatchesSet) {
     ImmutableMap.Builder<RegistrarThreatMatches, Throwable> failedMatchesBuilder =
         ImmutableMap.builder();
     for (RegistrarThreatMatches registrarThreatMatches : registrarThreatMatchesSet) {
+      RegistrarThreatMatches filteredMatches = filterOutNonPublishedMatches(registrarThreatMatches);
+      if (!filteredMatches.threatMatches().isEmpty()) {
         try {
-        // Handle exceptions individually per registrar so that one failed email doesn't prevent the
+          // Handle exceptions individually per registrar so that one failed email doesn't prevent
-        // rest from being sent.
+          // the rest from being sent.
-        emailRegistrar(date, soyTemplateInfo, subject, registrarThreatMatches);
+          emailRegistrar(date, soyTemplateInfo, subject, filteredMatches);
         } catch (Throwable e) {
           failedMatchesBuilder.put(registrarThreatMatches, getRootCause(e));
         }
       }
+    }
     ImmutableMap<RegistrarThreatMatches, Throwable> failedMatches = failedMatchesBuilder.build();
     if (!failedMatches.isEmpty()) {
       ImmutableList<Map.Entry<RegistrarThreatMatches, Throwable>> failedMatchesList =
@@ -120,6 +126,16 @@ public class Spec11EmailUtils {
         "Spec11 reporting completed successfully.");
   }
 
+  private RegistrarThreatMatches filterOutNonPublishedMatches(
+      RegistrarThreatMatches registrarThreatMatches) {
+    ImmutableList<ThreatMatch> filteredMatches = registrarThreatMatches.threatMatches().stream()
+        .filter(threatMatch ->
+            ofy().load().type(DomainBase.class).filter("fullyQualifiedDomainName",
+                threatMatch.fullyQualifiedDomainName()).first().now().shouldPublishToDns()
+        ).collect(toImmutableList());
+    return RegistrarThreatMatches.create(registrarThreatMatches.clientId(), filteredMatches);
+  }
+
   private void emailRegistrar(
       LocalDate date,
       SoyTemplateInfo soyTemplateInfo,

core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java

@@ -15,9 +15,15 @@
 package google.registry.reporting.spec11;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.eppcommon.StatusValue.CLIENT_HOLD;
+import static google.registry.model.eppcommon.StatusValue.SERVER_HOLD;
+import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchA;
 import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchB;
 import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.sampleThreatMatches;
+import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.newDomainBase;
+import static google.registry.testing.DatastoreHelper.persistActiveHost;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.JUnitBackports.assertThrows;
 import static org.mockito.Mockito.doThrow;
@@ -29,6 +35,9 @@ import static org.mockito.Mockito.when;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.net.MediaType;
+import com.googlecode.objectify.Key;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.host.HostResource;
 import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo;
 import google.registry.testing.AppEngineRule;
 import google.registry.util.EmailMessage;
@@ -118,6 +127,9 @@ public class Spec11EmailUtilsTest {
   private ArgumentCaptor<EmailMessage> contentCaptor;
   private final LocalDate date = new LocalDate(2018, 7, 15);
 
+  private DomainBase aDomain;
+  private DomainBase bDomain;
+
   @Before
   public void setUp() throws Exception {
     emailService = mock(SendEmailService.class);
@@ -132,6 +144,12 @@ public class Spec11EmailUtilsTest {
             new InternetAddress("my-reply-to@test.com"),
             FAKE_RESOURCES,
             "Super Cool Registry");
+
+    createTld("com");
+    HostResource host = persistActiveHost("ns1.example.com");
+    aDomain = persistDomainWithHost("a.com", host);
+    bDomain = persistDomainWithHost("b.com", host);
+    persistDomainWithHost("c.com", host);
   }
 
   @Test
@@ -210,6 +228,41 @@ public class Spec11EmailUtilsTest {
         Optional.empty());
   }
 
+  @Test
+  public void testSuccess_skipsInactiveDomain() throws Exception {
+    // CLIENT_HOLD and SERVER_HOLD mean no DNS so we don't need to email it out
+    persistResource(
+        ofy().load().entity(aDomain).now().asBuilder().addStatusValue(SERVER_HOLD)
+            .build());
+    persistResource(
+        ofy().load().entity(bDomain).now().asBuilder().addStatusValue(CLIENT_HOLD)
+            .build());
+    emailUtils.emailSpec11Reports(
+        date,
+        Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL,
+        "Super Cool Registry Monthly Threat Detector [2018-07-15]",
+        sampleThreatMatches());
+    // We inspect individual parameters because Message doesn't implement equals().
+    verify(emailService, times(2)).sendEmail(contentCaptor.capture());
+    List<EmailMessage> capturedContents = contentCaptor.getAllValues();
+    validateMessage(
+        capturedContents.get(0),
+        "my-sender@test.com",
+        "new.registrar@example.com",
+        Optional.of("my-reply-to@test.com"),
+        "Super Cool Registry Monthly Threat Detector [2018-07-15]",
+        String.format(MONTHLY_EMAIL_FORMAT, "<tr><td>c.com</td><td>MALWARE</td></tr>"),
+        Optional.of(MediaType.HTML_UTF_8));
+    validateMessage(
+        capturedContents.get(1),
+        "my-sender@test.com",
+        "my-receiver@test.com",
+        Optional.empty(),
+        "Spec11 Pipeline Success 2018-07-15",
+        "Spec11 reporting completed successfully.",
+        Optional.empty());
+  }
+
   @Test
   public void testOneFailure_sendsAlert() throws Exception {
     // If there is one failure, we should still send the other message and then an alert email
@@ -229,7 +282,7 @@ public class Spec11EmailUtilsTest {
                     date,
                     Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL,
                     "Super Cool Registry Monthly Threat Detector [2018-07-15]",
-                    matches));
+                    ImmutableSet.copyOf(matches)));
     assertThat(thrown)
         .hasMessageThat()
         .isEqualTo("Emailing Spec11 reports failed, first exception:");
@@ -340,4 +393,10 @@ public class Spec11EmailUtilsTest {
     contentType.ifPresent(expectedContentBuilder::setContentType);
     assertThat(message).isEqualTo(expectedContentBuilder.build());
   }
+
+  private static DomainBase persistDomainWithHost(String domainName, HostResource host) {
+    return persistResource(
+        newDomainBase(domainName).asBuilder().setNameservers(ImmutableSet.of(Key.create(host)))
+            .build());
+  }
 }