From 3e0eaecc9b85dbc9f681df02da268833daac689c Mon Sep 17 00:00:00 2001
From: guyben <guyben@google.com>
Date: Tue, 15 Jan 2019 12:56:17 -0800
Subject: [PATCH] Remove @Parameter from RdeStagingReducer

The "lenient" bit must be the same between RdeStagingMapper and
RdeStagingReducer, but this is hidden by the Reducer receiving the bit in a
completely different way than the mapper.

There are 2 ways to do this:
- add a "setLenient" function to RdeStagingReducer that we MUST call, or else
  get a runtime error. This is the simplest solution
- have a RdeStagingReducerBuilder you can inject, and that requires the
  "lenient" value to actually build the RdeStagingReducer. This prevents bugs
  at compile-time but is "more complicated"

I'm going with the second one here, but feel free to ask for the first one.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229423590
---
 .../google/registry/rde/RdeStagingAction.java |  7 ++-
 .../registry/rde/RdeStagingReducer.java       | 43 ++++++++++++++-----
 .../registry/rde/RdeStagingActionTest.java    | 15 +++----
 3 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/java/google/registry/rde/RdeStagingAction.java b/java/google/registry/rde/RdeStagingAction.java
index 790ede76d..96199dfa3 100644
--- a/java/google/registry/rde/RdeStagingAction.java
+++ b/java/google/registry/rde/RdeStagingAction.java
@@ -45,6 +45,7 @@ import google.registry.request.RequestParameters;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
 import google.registry.util.Clock;
+import google.registry.xml.ValidationMode;
 import java.util.Optional;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
@@ -203,7 +204,7 @@ public final class RdeStagingAction implements Runnable {
 
   @Inject Clock clock;
   @Inject PendingDepositChecker pendingDepositChecker;
-  @Inject RdeStagingReducer reducer;
+  @Inject RdeStagingReducer.Factory reducerFactory;
   @Inject Response response;
   @Inject MapreduceRunner mrRunner;
   @Inject @Config("transactionCooldown") Duration transactionCooldown;
@@ -231,7 +232,9 @@ public final class RdeStagingAction implements Runnable {
     for (PendingDeposit pending : pendings.values()) {
       logger.atInfo().log("Pending deposit: %s", pending);
     }
-    RdeStagingMapper mapper = new RdeStagingMapper(lenient ? LENIENT : STRICT, pendings);
+    ValidationMode validationMode = lenient ? LENIENT : STRICT;
+    RdeStagingMapper mapper = new RdeStagingMapper(validationMode, pendings);
+    RdeStagingReducer reducer = reducerFactory.create(validationMode);
 
     mrRunner
         .setJobName("Stage escrow deposits for all TLDs")
diff --git a/java/google/registry/rde/RdeStagingReducer.java b/java/google/registry/rde/RdeStagingReducer.java
index 3adb9931c..21688463b 100644
--- a/java/google/registry/rde/RdeStagingReducer.java
+++ b/java/google/registry/rde/RdeStagingReducer.java
@@ -21,8 +21,6 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Verify.verify;
 import static google.registry.model.common.Cursor.getCursorTimeOrStartOfTime;
 import static google.registry.model.ofy.ObjectifyService.ofy;
-import static google.registry.xml.ValidationMode.LENIENT;
-import static google.registry.xml.ValidationMode.STRICT;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.appengine.tools.cloudstorage.GcsFilename;
@@ -39,13 +37,13 @@ import google.registry.model.rde.RdeMode;
 import google.registry.model.rde.RdeNamingUtils;
 import google.registry.model.rde.RdeRevision;
 import google.registry.model.registry.Registry;
-import google.registry.request.Parameter;
 import google.registry.request.RequestParameters;
 import google.registry.request.lock.LockHandler;
 import google.registry.tldconfig.idn.IdnTableEnum;
 import google.registry.util.TaskQueueUtils;
 import google.registry.xjc.rdeheader.XjcRdeHeader;
 import google.registry.xjc.rdeheader.XjcRdeHeaderElement;
+import google.registry.xml.ValidationMode;
 import google.registry.xml.XmlException;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -76,22 +74,21 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
   private final byte[] stagingKeyBytes;
   private final RdeMarshaller marshaller;
 
-  @Inject
-  RdeStagingReducer(
+  private RdeStagingReducer(
       TaskQueueUtils taskQueueUtils,
       LockHandler lockHandler,
-      @Config("gcsBufferSize") int gcsBufferSize,
-      @Config("rdeBucket") String bucket,
-      @Config("rdeStagingLockTimeout") Duration lockTimeout,
-      @KeyModule.Key("rdeStagingEncryptionKey") byte[] stagingKeyBytes,
-      @Parameter(RdeModule.PARAM_LENIENT) boolean lenient) {
+      int gcsBufferSize,
+      String bucket,
+      Duration lockTimeout,
+      byte[] stagingKeyBytes,
+      ValidationMode validationMode) {
     this.taskQueueUtils = taskQueueUtils;
     this.lockHandler = lockHandler;
     this.gcsBufferSize = gcsBufferSize;
     this.bucket = bucket;
     this.lockTimeout = lockTimeout;
     this.stagingKeyBytes = stagingKeyBytes;
-    this.marshaller = new RdeMarshaller(lenient ? LENIENT : STRICT);
+    this.marshaller = new RdeMarshaller(validationMode);
   }
 
   @Override
@@ -240,4 +237,28 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
               }
             });
   }
+
+  /** Injectible factory for creating {@link RdeStagingReducer}. */
+  static class Factory {
+    TaskQueueUtils taskQueueUtils;
+    LockHandler lockHandler;
+    @Config("gcsBufferSize") int gcsBufferSize;
+    @Config("rdeBucket") String bucket;
+    @Config("rdeStagingLockTimeout") Duration lockTimeout;
+    @KeyModule.Key("rdeStagingEncryptionKey") byte[] stagingKeyBytes;
+
+    @Inject
+    Factory() {}
+
+    RdeStagingReducer create(ValidationMode validationMode) {
+      return new RdeStagingReducer(
+          taskQueueUtils,
+          lockHandler,
+          gcsBufferSize,
+          bucket,
+          lockTimeout,
+          stagingKeyBytes,
+          validationMode);
+    }
+  }
 }
diff --git a/javatests/google/registry/rde/RdeStagingActionTest.java b/javatests/google/registry/rde/RdeStagingActionTest.java
index cc7e3f8a2..4e045ff00 100644
--- a/javatests/google/registry/rde/RdeStagingActionTest.java
+++ b/javatests/google/registry/rde/RdeStagingActionTest.java
@@ -131,14 +131,13 @@ public class RdeStagingActionTest extends MapreduceTestCase<RdeStagingAction> {
     action.clock = clock;
     action.mrRunner = makeDefaultRunner();
     action.lenient = false;
-    action.reducer = new RdeStagingReducer(
-        new TaskQueueUtils(new Retrier(new SystemSleeper(), 1)), // taskQueueUtils
-        new FakeLockHandler(true),
-        0, // gcsBufferSize
-        "rde-bucket", // bucket
-        Duration.standardHours(1), // lockTimeout
-        PgpHelper.convertPublicKeyToBytes(encryptKey), // stagingKeyBytes
-        false); // lenient
+    action.reducerFactory = new RdeStagingReducer.Factory();
+    action.reducerFactory.taskQueueUtils = new TaskQueueUtils(new Retrier(new SystemSleeper(), 1));
+    action.reducerFactory.lockHandler = new FakeLockHandler(true);
+    action.reducerFactory.gcsBufferSize = 0;
+    action.reducerFactory.bucket = "rde-bucket";
+    action.reducerFactory.lockTimeout = Duration.standardHours(1);
+    action.reducerFactory.stagingKeyBytes = PgpHelper.convertPublicKeyToBytes(encryptKey);
     action.pendingDepositChecker = new PendingDepositChecker();
     action.pendingDepositChecker.brdaDayOfWeek = DateTimeConstants.TUESDAY;
     action.pendingDepositChecker.brdaInterval = Duration.standardDays(7);