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
This commit is contained in:
guyben 2019-01-15 12:56:17 -08:00 committed by jianglai
parent 035b9149e0
commit 3e0eaecc9b
3 changed files with 44 additions and 21 deletions

View file

@ -45,6 +45,7 @@ import google.registry.request.RequestParameters;
import google.registry.request.Response; import google.registry.request.Response;
import google.registry.request.auth.Auth; import google.registry.request.auth.Auth;
import google.registry.util.Clock; import google.registry.util.Clock;
import google.registry.xml.ValidationMode;
import java.util.Optional; import java.util.Optional;
import javax.inject.Inject; import javax.inject.Inject;
import org.joda.time.DateTime; import org.joda.time.DateTime;
@ -203,7 +204,7 @@ public final class RdeStagingAction implements Runnable {
@Inject Clock clock; @Inject Clock clock;
@Inject PendingDepositChecker pendingDepositChecker; @Inject PendingDepositChecker pendingDepositChecker;
@Inject RdeStagingReducer reducer; @Inject RdeStagingReducer.Factory reducerFactory;
@Inject Response response; @Inject Response response;
@Inject MapreduceRunner mrRunner; @Inject MapreduceRunner mrRunner;
@Inject @Config("transactionCooldown") Duration transactionCooldown; @Inject @Config("transactionCooldown") Duration transactionCooldown;
@ -231,7 +232,9 @@ public final class RdeStagingAction implements Runnable {
for (PendingDeposit pending : pendings.values()) { for (PendingDeposit pending : pendings.values()) {
logger.atInfo().log("Pending deposit: %s", pending); 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 mrRunner
.setJobName("Stage escrow deposits for all TLDs") .setJobName("Stage escrow deposits for all TLDs")

View file

@ -21,8 +21,6 @@ import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Verify.verify; import static com.google.common.base.Verify.verify;
import static google.registry.model.common.Cursor.getCursorTimeOrStartOfTime; import static google.registry.model.common.Cursor.getCursorTimeOrStartOfTime;
import static google.registry.model.ofy.ObjectifyService.ofy; 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 static java.nio.charset.StandardCharsets.UTF_8;
import com.google.appengine.tools.cloudstorage.GcsFilename; 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.RdeNamingUtils;
import google.registry.model.rde.RdeRevision; import google.registry.model.rde.RdeRevision;
import google.registry.model.registry.Registry; import google.registry.model.registry.Registry;
import google.registry.request.Parameter;
import google.registry.request.RequestParameters; import google.registry.request.RequestParameters;
import google.registry.request.lock.LockHandler; import google.registry.request.lock.LockHandler;
import google.registry.tldconfig.idn.IdnTableEnum; import google.registry.tldconfig.idn.IdnTableEnum;
import google.registry.util.TaskQueueUtils; import google.registry.util.TaskQueueUtils;
import google.registry.xjc.rdeheader.XjcRdeHeader; import google.registry.xjc.rdeheader.XjcRdeHeader;
import google.registry.xjc.rdeheader.XjcRdeHeaderElement; import google.registry.xjc.rdeheader.XjcRdeHeaderElement;
import google.registry.xml.ValidationMode;
import google.registry.xml.XmlException; import google.registry.xml.XmlException;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
@ -76,22 +74,21 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
private final byte[] stagingKeyBytes; private final byte[] stagingKeyBytes;
private final RdeMarshaller marshaller; private final RdeMarshaller marshaller;
@Inject private RdeStagingReducer(
RdeStagingReducer(
TaskQueueUtils taskQueueUtils, TaskQueueUtils taskQueueUtils,
LockHandler lockHandler, LockHandler lockHandler,
@Config("gcsBufferSize") int gcsBufferSize, int gcsBufferSize,
@Config("rdeBucket") String bucket, String bucket,
@Config("rdeStagingLockTimeout") Duration lockTimeout, Duration lockTimeout,
@KeyModule.Key("rdeStagingEncryptionKey") byte[] stagingKeyBytes, byte[] stagingKeyBytes,
@Parameter(RdeModule.PARAM_LENIENT) boolean lenient) { ValidationMode validationMode) {
this.taskQueueUtils = taskQueueUtils; this.taskQueueUtils = taskQueueUtils;
this.lockHandler = lockHandler; this.lockHandler = lockHandler;
this.gcsBufferSize = gcsBufferSize; this.gcsBufferSize = gcsBufferSize;
this.bucket = bucket; this.bucket = bucket;
this.lockTimeout = lockTimeout; this.lockTimeout = lockTimeout;
this.stagingKeyBytes = stagingKeyBytes; this.stagingKeyBytes = stagingKeyBytes;
this.marshaller = new RdeMarshaller(lenient ? LENIENT : STRICT); this.marshaller = new RdeMarshaller(validationMode);
} }
@Override @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);
}
}
} }

View file

@ -131,14 +131,13 @@ public class RdeStagingActionTest extends MapreduceTestCase<RdeStagingAction> {
action.clock = clock; action.clock = clock;
action.mrRunner = makeDefaultRunner(); action.mrRunner = makeDefaultRunner();
action.lenient = false; action.lenient = false;
action.reducer = new RdeStagingReducer( action.reducerFactory = new RdeStagingReducer.Factory();
new TaskQueueUtils(new Retrier(new SystemSleeper(), 1)), // taskQueueUtils action.reducerFactory.taskQueueUtils = new TaskQueueUtils(new Retrier(new SystemSleeper(), 1));
new FakeLockHandler(true), action.reducerFactory.lockHandler = new FakeLockHandler(true);
0, // gcsBufferSize action.reducerFactory.gcsBufferSize = 0;
"rde-bucket", // bucket action.reducerFactory.bucket = "rde-bucket";
Duration.standardHours(1), // lockTimeout action.reducerFactory.lockTimeout = Duration.standardHours(1);
PgpHelper.convertPublicKeyToBytes(encryptKey), // stagingKeyBytes action.reducerFactory.stagingKeyBytes = PgpHelper.convertPublicKeyToBytes(encryptKey);
false); // lenient
action.pendingDepositChecker = new PendingDepositChecker(); action.pendingDepositChecker = new PendingDepositChecker();
action.pendingDepositChecker.brdaDayOfWeek = DateTimeConstants.TUESDAY; action.pendingDepositChecker.brdaDayOfWeek = DateTimeConstants.TUESDAY;
action.pendingDepositChecker.brdaInterval = Duration.standardDays(7); action.pendingDepositChecker.brdaInterval = Duration.standardDays(7);