Decouple dryRun from SessionMetadata

dryRun is only available via the (sessionless!) tool, and is not
a property of the session.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125203026
This commit is contained in:
cgoldfeder 2016-06-17 14:31:22 -07:00 committed by Ben McIlwain
parent 3ae646d687
commit e359ab5f52
17 changed files with 31 additions and 54 deletions

View file

@ -43,6 +43,7 @@ public class EppConsoleAction implements Runnable {
eppRequestHandler.executeEpp( eppRequestHandler.executeEpp(
new HttpSessionMetadata(session), new HttpSessionMetadata(session),
new GaeUserCredentials(getUserService().getCurrentUser()), new GaeUserCredentials(getUserService().getCurrentUser()),
false, // This endpoint is never a dry run.
inputXmlBytes); inputXmlBytes);
} }
} }

View file

@ -54,6 +54,7 @@ public final class EppController {
public byte[] handleEppCommand( public byte[] handleEppCommand(
SessionMetadata sessionMetadata, SessionMetadata sessionMetadata,
TransportCredentials credentials, TransportCredentials credentials,
boolean isDryRun,
byte[] inputXmlBytes) { byte[] inputXmlBytes) {
Trid trid = null; Trid trid = null;
try { try {
@ -72,6 +73,7 @@ public final class EppController {
trid, trid,
sessionMetadata, sessionMetadata,
credentials, credentials,
isDryRun,
inputXmlBytes, inputXmlBytes,
metrics, metrics,
clock); clock);

View file

@ -42,10 +42,12 @@ public class EppRequestHandler {
public void executeEpp( public void executeEpp(
SessionMetadata sessionMetadata, SessionMetadata sessionMetadata,
TransportCredentials credentials, TransportCredentials credentials,
boolean isDryRun,
byte[] inputXmlBytes) { byte[] inputXmlBytes) {
try { try {
response.setPayload(new String( response.setPayload(new String(
eppController.handleEppCommand(sessionMetadata, credentials, inputXmlBytes), UTF_8)); eppController.handleEppCommand(
sessionMetadata, credentials, isDryRun, inputXmlBytes), UTF_8));
response.setContentType(APPLICATION_EPP_XML); response.setContentType(APPLICATION_EPP_XML);
// Note that we always return 200 (OK) even if the EppController returns an error response. // Note that we always return 200 (OK) even if the EppController returns an error response.
// This is because returning an non-OK HTTP status code will cause the proxy server to // This is because returning an non-OK HTTP status code will cause the proxy server to

View file

@ -46,7 +46,11 @@ public class EppTlsAction implements Runnable {
if (!tlsCredentials.hasSni()) { if (!tlsCredentials.hasSni()) {
logger.warning("Request did not include required SNI header."); logger.warning("Request did not include required SNI header.");
} }
eppRequestHandler.executeEpp(new HttpSessionMetadata(session), tlsCredentials, inputXmlBytes); eppRequestHandler.executeEpp(
new HttpSessionMetadata(session),
tlsCredentials,
false, // This endpoint is never a dry run.
inputXmlBytes);
} }
} }

View file

@ -51,10 +51,10 @@ public class EppToolAction implements Runnable {
new StatelessRequestSessionMetadata( new StatelessRequestSessionMetadata(
clientIdentifier, clientIdentifier,
superuser, superuser,
dryRun,
ProtocolDefinition.getVisibleServiceExtensionUris(), ProtocolDefinition.getVisibleServiceExtensionUris(),
SessionSource.TOOL), SessionSource.TOOL),
new PasswordOnlyTransportCredentials(), new PasswordOnlyTransportCredentials(),
dryRun,
xml.getBytes(UTF_8)); xml.getBytes(UTF_8));
} }

View file

@ -36,7 +36,7 @@ import org.joda.time.DateTime;
/** Run a flow, either transactionally or not, with logging and retrying as needed. */ /** Run a flow, either transactionally or not, with logging and retrying as needed. */
public class FlowRunner { public class FlowRunner {
private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 5); private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 6);
private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
@ -44,6 +44,7 @@ public class FlowRunner {
private final EppInput eppInput; private final EppInput eppInput;
private final Trid trid; private final Trid trid;
private final SessionMetadata sessionMetadata; private final SessionMetadata sessionMetadata;
private final boolean isDryRun;
private final TransportCredentials credentials; private final TransportCredentials credentials;
private final byte[] inputXmlBytes; private final byte[] inputXmlBytes;
private final EppMetrics metrics; private final EppMetrics metrics;
@ -55,6 +56,7 @@ public class FlowRunner {
Trid trid, Trid trid,
SessionMetadata sessionMetadata, SessionMetadata sessionMetadata,
TransportCredentials credentials, TransportCredentials credentials,
boolean isDryRun,
byte[] inputXmlBytes, byte[] inputXmlBytes,
final EppMetrics metrics, final EppMetrics metrics,
Clock clock) { Clock clock) {
@ -64,6 +66,7 @@ public class FlowRunner {
this.trid = trid; this.trid = trid;
this.sessionMetadata = sessionMetadata; this.sessionMetadata = sessionMetadata;
this.credentials = credentials; this.credentials = credentials;
this.isDryRun = isDryRun;
this.inputXmlBytes = inputXmlBytes; this.inputXmlBytes = inputXmlBytes;
this.metrics = metrics; this.metrics = metrics;
this.clock = clock; this.clock = clock;
@ -77,7 +80,8 @@ public class FlowRunner {
clientId, clientId,
sessionMetadata, sessionMetadata,
prettyPrint(inputXmlBytes).replaceAll("\n", "\n\t"), prettyPrint(inputXmlBytes).replaceAll("\n", "\n\t"),
credentials); credentials,
isDryRun ? "DRY_RUN" : "LIVE");
if (!isTransactional()) { if (!isTransactional()) {
if (metrics != null) { if (metrics != null) {
metrics.incrementAttempts(); metrics.incrementAttempts();
@ -100,7 +104,7 @@ public class FlowRunner {
} }
try { try {
EppOutput output = createAndInitFlow(ofy().getTransactionTime()).run(); EppOutput output = createAndInitFlow(ofy().getTransactionTime()).run();
if (sessionMetadata.isDryRun()) { if (isDryRun) {
throw new DryRunException(output); throw new DryRunException(output);
} }
return output; return output;

View file

@ -137,12 +137,6 @@ public abstract class SessionMetadata {
setPropertyChecked(FAILED_LOGIN_ATTEMPTS_KEY, null); setPropertyChecked(FAILED_LOGIN_ATTEMPTS_KEY, null);
} }
// These three methods are here to allow special permissions if a derived class overrides them.
public boolean isDryRun() {
return false;
}
@Override @Override
public String toString() { public String toString() {
return toStringHelper(getClass()) return toStringHelper(getClass())

View file

@ -21,19 +21,16 @@ public class StatelessRequestSessionMetadata extends SessionMetadata {
private final String clientId; private final String clientId;
private final boolean isSuperuser; private final boolean isSuperuser;
private final boolean isDryRun;
private final Set<String> serviceExtensionUris; private final Set<String> serviceExtensionUris;
private final SessionSource sessionSource; private final SessionSource sessionSource;
public StatelessRequestSessionMetadata( public StatelessRequestSessionMetadata(
String clientId, String clientId,
boolean isSuperuser, boolean isSuperuser,
boolean isDryRun,
Set<String> serviceExtensionUris, Set<String> serviceExtensionUris,
SessionSource source) { SessionSource source) {
this.clientId = clientId; this.clientId = clientId;
this.isSuperuser = isSuperuser; this.isSuperuser = isSuperuser;
this.isDryRun = isDryRun;
this.serviceExtensionUris = serviceExtensionUris; this.serviceExtensionUris = serviceExtensionUris;
this.sessionSource = source; this.sessionSource = source;
} }
@ -48,11 +45,6 @@ public class StatelessRequestSessionMetadata extends SessionMetadata {
return isSuperuser; return isSuperuser;
} }
@Override
public boolean isDryRun() {
return isDryRun;
}
@Override @Override
public Set<String> getServiceExtensionUris() { public Set<String> getServiceExtensionUris() {
return serviceExtensionUris; return serviceExtensionUris;

View file

@ -109,6 +109,7 @@ final class ValidateLoginCredentialsCommand implements RemoteApiCommand, GtechCo
clientCertificateHash, clientCertificateHash,
Optional.of(clientIpAddress), Optional.of(clientIpAddress),
"placeholder"), // behave as if we have SNI on, since we're validating a cert "placeholder"), // behave as if we have SNI on, since we're validating a cert
false,
inputXmlBytes, inputXmlBytes,
null, null,
new SystemClock()).run()), UTF_8)); new SystemClock()).run()), UTF_8));

View file

@ -81,7 +81,6 @@ public class CheckApiAction implements Runnable {
new StatelessRequestSessionMetadata( new StatelessRequestSessionMetadata(
RegistryEnvironment.get().config().getCheckApiServletRegistrarClientId(), RegistryEnvironment.get().config().getCheckApiServletRegistrarClientId(),
false, false,
false,
ImmutableSet.of(FEE_0_6.getUri()), ImmutableSet.of(FEE_0_6.getUri()),
SessionSource.HTTP); SessionSource.HTTP);
@ -121,6 +120,7 @@ public class CheckApiAction implements Runnable {
Trid.create(getClass().getSimpleName()), Trid.create(getClass().getSimpleName()),
sessionMetadata, sessionMetadata,
new PasswordOnlyTransportCredentials(), new PasswordOnlyTransportCredentials(),
false,
inputXmlBytes, inputXmlBytes,
null, null,
clock) clock)

View file

@ -55,12 +55,11 @@ public class EppConsoleActionTest extends ShardableTestCase {
ArgumentCaptor.forClass(TransportCredentials.class); ArgumentCaptor.forClass(TransportCredentials.class);
ArgumentCaptor<SessionMetadata> metadataCaptor = ArgumentCaptor.forClass(SessionMetadata.class); ArgumentCaptor<SessionMetadata> metadataCaptor = ArgumentCaptor.forClass(SessionMetadata.class);
verify(action.eppRequestHandler).executeEpp( verify(action.eppRequestHandler).executeEpp(
metadataCaptor.capture(), credentialsCaptor.capture(), eq(INPUT_XML_BYTES)); metadataCaptor.capture(), credentialsCaptor.capture(), eq(false), eq(INPUT_XML_BYTES));
assertThat(((GaeUserCredentials) credentialsCaptor.getValue()).gaeUser.getEmail()) assertThat(((GaeUserCredentials) credentialsCaptor.getValue()).gaeUser.getEmail())
.isEqualTo("person@example.com"); .isEqualTo("person@example.com");
SessionMetadata sessionMetadata = metadataCaptor.getValue(); SessionMetadata sessionMetadata = metadataCaptor.getValue();
assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier"); assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier");
assertThat(sessionMetadata.isDryRun()).isFalse(); // Should always be false for console.
assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser); assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser);
} }

View file

@ -118,7 +118,7 @@ public class EppTestCase extends ShardableTestCase {
handler.eppController = new EppController(); handler.eppController = new EppController();
handler.eppController.clock = clock; handler.eppController.clock = clock;
handler.eppController.metrics = mock(EppMetrics.class); handler.eppController.metrics = mock(EppMetrics.class);
handler.executeEpp(sessionMetadata, credentials, inputXml.getBytes(UTF_8)); handler.executeEpp(sessionMetadata, credentials, false, inputXml.getBytes(UTF_8));
assertThat(response.getStatus()).isEqualTo(SC_OK); assertThat(response.getStatus()).isEqualTo(SC_OK);
assertThat(response.getContentType()).isEqualTo(APPLICATION_EPP_XML_UTF8); assertThat(response.getContentType()).isEqualTo(APPLICATION_EPP_XML_UTF8);
String result = response.getPayload(); String result = response.getPayload();

View file

@ -49,10 +49,9 @@ public class EppTlsActionTest extends ShardableTestCase {
action.run(); action.run();
ArgumentCaptor<SessionMetadata> captor = ArgumentCaptor.forClass(SessionMetadata.class); ArgumentCaptor<SessionMetadata> captor = ArgumentCaptor.forClass(SessionMetadata.class);
verify(action.eppRequestHandler) verify(action.eppRequestHandler)
.executeEpp(captor.capture(), same(action.tlsCredentials), eq(INPUT_XML_BYTES)); .executeEpp(captor.capture(), same(action.tlsCredentials), eq(false), eq(INPUT_XML_BYTES));
SessionMetadata sessionMetadata = captor.getValue(); SessionMetadata sessionMetadata = captor.getValue();
assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier"); assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier");
assertThat(sessionMetadata.isDryRun()).isFalse(); // Should always be false for TLS.
assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser); assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser);
} }

View file

@ -42,10 +42,10 @@ public class EppToolActionTest {
verify(action.eppRequestHandler).executeEpp( verify(action.eppRequestHandler).executeEpp(
captor.capture(), captor.capture(),
isA(PasswordOnlyTransportCredentials.class), isA(PasswordOnlyTransportCredentials.class),
eq(dryRun),
eq(action.xml.getBytes(UTF_8))); eq(action.xml.getBytes(UTF_8)));
SessionMetadata sessionMetadata = captor.getValue(); SessionMetadata sessionMetadata = captor.getValue();
assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier"); assertThat(sessionMetadata.getClientId()).isEqualTo("ClientIdentifier");
assertThat(sessionMetadata.isDryRun()).isEqualTo(dryRun);
assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser); assertThat(sessionMetadata.isSuperuser()).isEqualTo(superuser);
} }

View file

@ -87,8 +87,6 @@ public abstract class FlowTestCase<F extends Flow> {
@Rule @Rule
public final InjectRule inject = new InjectRule(); public final InjectRule inject = new InjectRule();
private FlowRunner flowRunner;
protected EppLoader eppLoader; protected EppLoader eppLoader;
protected Class<? extends Flow> flowClass; protected Class<? extends Flow> flowClass;
protected TestSessionMetadata sessionMetadata; protected TestSessionMetadata sessionMetadata;
@ -97,7 +95,6 @@ public abstract class FlowTestCase<F extends Flow> {
@Before @Before
public void init() throws Exception { public void init() throws Exception {
flowRunner = null;
sessionMetadata = new TestSessionMetadata(); sessionMetadata = new TestSessionMetadata();
sessionMetadata.setClientId("TheRegistrar"); sessionMetadata.setClientId("TheRegistrar");
sessionMetadata.setServiceExtensionUris(ProtocolDefinition.getVisibleServiceExtensionUris()); sessionMetadata.setServiceExtensionUris(ProtocolDefinition.getVisibleServiceExtensionUris());
@ -123,16 +120,8 @@ public abstract class FlowTestCase<F extends Flow> {
return readResourceUtf8(getClass(), "testdata/" + filename); return readResourceUtf8(getClass(), "testdata/" + filename);
} }
/** Lazily load the flow, since it may fail to initialize if the environment isn't set up yet. */
public FlowRunner getFlowRunner() throws Exception {
if (flowRunner == null) {
flowRunner = createFlowRunner();
}
return flowRunner;
}
/** Load a flow from an epp object. */ /** Load a flow from an epp object. */
private FlowRunner createFlowRunner() throws Exception { private FlowRunner getFlowRunner(CommitMode commitMode) throws Exception {
EppInput eppInput = eppLoader.getEpp(); EppInput eppInput = eppLoader.getEpp();
flowClass = firstNonNull(flowClass, FlowPicker.getFlowClass(eppInput)); flowClass = firstNonNull(flowClass, FlowPicker.getFlowClass(eppInput));
Class<?> expectedFlowClass = new TypeInstantiator<F>(getClass()){}.getExactType(); Class<?> expectedFlowClass = new TypeInstantiator<F>(getClass()){}.getExactType();
@ -143,6 +132,7 @@ public abstract class FlowTestCase<F extends Flow> {
getTrid(), getTrid(),
sessionMetadata, sessionMetadata,
credentials, credentials,
commitMode.equals(CommitMode.DRY_RUN),
"<xml></xml>".getBytes(), "<xml></xml>".getBytes(),
null, null,
clock); clock);
@ -163,7 +153,7 @@ public abstract class FlowTestCase<F extends Flow> {
} }
public void assertTransactionalFlow(boolean isTransactional) throws Exception { public void assertTransactionalFlow(boolean isTransactional) throws Exception {
assertThat(getFlowRunner().isTransactional()).isEqualTo(isTransactional); assertThat(getFlowRunner(CommitMode.LIVE).isTransactional()).isEqualTo(isTransactional);
} }
public void assertNoHistory() throws Exception { public void assertNoHistory() throws Exception {
@ -282,8 +272,7 @@ public abstract class FlowTestCase<F extends Flow> {
/** Run a flow, and attempt to marshal the result to EPP or throw if it doesn't validate. */ /** Run a flow, and attempt to marshal the result to EPP or throw if it doesn't validate. */
public EppOutput runFlow(CommitMode commitMode, UserPrivileges userPrivileges) throws Exception { public EppOutput runFlow(CommitMode commitMode, UserPrivileges userPrivileges) throws Exception {
sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER)); sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER));
sessionMetadata.setIsDryRun(commitMode.equals(CommitMode.DRY_RUN)); EppOutput output = getFlowRunner(commitMode).run();
EppOutput output = getFlowRunner().run();
marshal(output, ValidationMode.STRICT); marshal(output, ValidationMode.STRICT);
return output; return output;
} }
@ -296,8 +285,7 @@ public abstract class FlowTestCase<F extends Flow> {
CommitMode commitMode, UserPrivileges userPrivileges, String xml, String... ignoredPaths) CommitMode commitMode, UserPrivileges userPrivileges, String xml, String... ignoredPaths)
throws Exception { throws Exception {
sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER)); sessionMetadata.setSuperuser(userPrivileges.equals(UserPrivileges.SUPERUSER));
sessionMetadata.setIsDryRun(commitMode.equals(CommitMode.DRY_RUN)); EppOutput eppOutput = getFlowRunner(commitMode).run();
EppOutput eppOutput = getFlowRunner().run();
if (eppOutput.isResponse()) { if (eppOutput.isResponse()) {
assertThat(eppOutput.isSuccess()).isTrue(); assertThat(eppOutput.isSuccess()).isTrue();
} }

View file

@ -86,6 +86,7 @@ public class EppResourceUtilsTest {
Trid.create(null, "server-trid"), Trid.create(null, "server-trid"),
sessionMetadata, sessionMetadata,
new PasswordOnlyTransportCredentials(), new PasswordOnlyTransportCredentials(),
false,
"<xml></xml>".getBytes(), "<xml></xml>".getBytes(),
null, null,
clock) clock)

View file

@ -25,7 +25,6 @@ public class TestSessionMetadata extends SessionMetadata {
private final Map<String, Object> properties = new HashMap<>(); private final Map<String, Object> properties = new HashMap<>();
private boolean isValid = true; private boolean isValid = true;
private boolean isDryRun = false;
private SessionSource sessionSource = SessionSource.NONE; private SessionSource sessionSource = SessionSource.NONE;
@Override @Override
@ -58,15 +57,6 @@ public class TestSessionMetadata extends SessionMetadata {
sessionSource = source; sessionSource = source;
} }
public void setIsDryRun(boolean isDryRun) {
this.isDryRun = isDryRun;
}
@Override
public boolean isDryRun() {
return isDryRun;
}
public boolean isValid() { public boolean isValid() {
return isValid; return isValid;
} }