diff --git a/java/google/registry/flows/FlowRunner.java b/java/google/registry/flows/FlowRunner.java index f7f0b9d71..e56d829d9 100644 --- a/java/google/registry/flows/FlowRunner.java +++ b/java/google/registry/flows/FlowRunner.java @@ -25,6 +25,7 @@ import google.registry.flows.FlowModule.DryRun; import google.registry.flows.FlowModule.InputXml; import google.registry.flows.FlowModule.Superuser; import google.registry.flows.FlowModule.Transactional; +import google.registry.flows.session.LoginFlow; import google.registry.model.eppcommon.Trid; import google.registry.model.eppoutput.EppOutput; import google.registry.monitoring.whitebox.EppMetric; @@ -80,7 +81,12 @@ public class FlowRunner { eppMetricBuilder.setCommandNameFromFlow(flowClass.getSimpleName()); if (!isTransactional) { eppMetricBuilder.incrementAttempts(); - return EppOutput.create(flowProvider.get().run()); + EppOutput eppOutput = EppOutput.create(flowProvider.get().run()); + if (flowClass.equals(LoginFlow.class)) { + // In LoginFlow, clientId isn't known until after the flow executes, so save it then. + eppMetricBuilder.setClientId(sessionMetadata.getClientId()); + } + return eppOutput; } try { return ofy().transact(new Work() { diff --git a/javatests/google/registry/flows/CheckApiActionTest.java b/javatests/google/registry/flows/CheckApiActionTest.java index 6a24bf358..b15801662 100644 --- a/javatests/google/registry/flows/CheckApiActionTest.java +++ b/javatests/google/registry/flows/CheckApiActionTest.java @@ -61,7 +61,7 @@ public class CheckApiActionTest { action.response = new FakeResponse(); action.checkApiServletRegistrarClientId = "TheRegistrar"; action.eppController = DaggerEppTestComponent.builder() - .fakesAndMocksModule(new FakesAndMocksModule()) + .fakesAndMocksModule(FakesAndMocksModule.create()) .build() .startRequest() .eppController(); diff --git a/javatests/google/registry/flows/EppCommitLogsTest.java b/javatests/google/registry/flows/EppCommitLogsTest.java index d98974088..6685167f4 100644 --- a/javatests/google/registry/flows/EppCommitLogsTest.java +++ b/javatests/google/registry/flows/EppCommitLogsTest.java @@ -73,19 +73,22 @@ public class EppCommitLogsTest extends ShardableTestCase { SessionMetadata sessionMetadata = new HttpSessionMetadata(new FakeHttpSession()); sessionMetadata.setClientId("TheRegistrar"); DaggerEppTestComponent.builder() - .fakesAndMocksModule(new FakesAndMocksModule(clock, TmchCaMode.PILOT)) + .fakesAndMocksModule( + FakesAndMocksModule.create( + clock, TmchCaMode.PILOT, EppMetric.builderForRequest("request-id-1", clock))) .build() .startRequest() .flowComponentBuilder() - .flowModule(new FlowModule.Builder() - .setSessionMetadata(sessionMetadata) - .setCredentials(new PasswordOnlyTransportCredentials()) - .setEppRequestSource(EppRequestSource.UNIT_TEST) - .setIsDryRun(false) - .setIsSuperuser(false) - .setInputXmlBytes(eppLoader.getEppXml().getBytes(UTF_8)) - .setEppInput(eppLoader.getEpp()) - .build()) + .flowModule( + new FlowModule.Builder() + .setSessionMetadata(sessionMetadata) + .setCredentials(new PasswordOnlyTransportCredentials()) + .setEppRequestSource(EppRequestSource.UNIT_TEST) + .setIsDryRun(false) + .setIsSuperuser(false) + .setInputXmlBytes(eppLoader.getEppXml().getBytes(UTF_8)) + .setEppInput(eppLoader.getEpp()) + .build()) .build() .flowRunner() .run(EppMetric.builder()); diff --git a/javatests/google/registry/flows/EppLifecycleLoginTest.java b/javatests/google/registry/flows/EppLifecycleLoginTest.java new file mode 100644 index 000000000..7a8bf4a15 --- /dev/null +++ b/javatests/google/registry/flows/EppLifecycleLoginTest.java @@ -0,0 +1,40 @@ +// Copyright 2017 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.flows; + +import static com.google.common.truth.Truth.assertThat; + +import google.registry.testing.AppEngineRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for login lifecycle. */ +@RunWith(JUnit4.class) +public class EppLifecycleLoginTest extends EppTestCase { + + @Rule + public final AppEngineRule appEngine = + AppEngineRule.builder().withDatastore().withTaskQueue().build(); + + @Test + public void testLoginAndLogout_recordClientIdInEppMetric() throws Exception { + assertCommandAndResponse("login_valid.xml", "login_response.xml"); + assertThat(getRecordedEppMetric().getClientId()).hasValue("NewRegistrar"); + assertCommandAndResponse("logout.xml", "logout_response.xml"); + assertThat(getRecordedEppMetric().getClientId()).hasValue("NewRegistrar"); + } +} diff --git a/javatests/google/registry/flows/EppTestCase.java b/javatests/google/registry/flows/EppTestCase.java index 7294e1198..327dd4832 100644 --- a/javatests/google/registry/flows/EppTestCase.java +++ b/javatests/google/registry/flows/EppTestCase.java @@ -26,6 +26,7 @@ import com.google.common.net.MediaType; import google.registry.config.RegistryConfig.ConfigModule.TmchCaMode; import google.registry.flows.EppTestComponent.FakesAndMocksModule; import google.registry.model.ofy.Ofy; +import google.registry.monitoring.whitebox.EppMetric; import google.registry.testing.FakeClock; import google.registry.testing.FakeHttpSession; import google.registry.testing.FakeResponse; @@ -48,6 +49,7 @@ public class EppTestCase extends ShardableTestCase { private SessionMetadata sessionMetadata; private TransportCredentials credentials = new PasswordOnlyTransportCredentials(); + private EppMetric.Builder eppMetricBuilder; private boolean isSuperuser; @Before @@ -114,8 +116,9 @@ public class EppTestCase extends ShardableTestCase { EppRequestHandler handler = new EppRequestHandler(); FakeResponse response = new FakeResponse(); handler.response = response; + eppMetricBuilder = EppMetric.builderForRequest("request-id-1", clock); handler.eppController = DaggerEppTestComponent.builder() - .fakesAndMocksModule(new FakesAndMocksModule(clock, TmchCaMode.PILOT)) + .fakesAndMocksModule(FakesAndMocksModule.create(clock, TmchCaMode.PILOT, eppMetricBuilder)) .build() .startRequest() .eppController(); @@ -133,4 +136,8 @@ public class EppTestCase extends ShardableTestCase { EppXmlTransformer.validateOutput(result); return result; } + + protected EppMetric getRecordedEppMetric() { + return eppMetricBuilder.build(); + } } diff --git a/javatests/google/registry/flows/EppTestComponent.java b/javatests/google/registry/flows/EppTestComponent.java index 5fbad9b57..cad904756 100644 --- a/javatests/google/registry/flows/EppTestComponent.java +++ b/javatests/google/registry/flows/EppTestComponent.java @@ -53,27 +53,31 @@ interface EppTestComponent { @Module static class FakesAndMocksModule { - final BigQueryMetricsEnqueuer metricsEnqueuer; - final DnsQueue dnsQueue; - final DomainFlowTmchUtils domainFlowTmchUtils; - final EppMetric.Builder metricBuilder; - final FakeClock clock; - final ModulesService modulesService; - final Sleeper sleeper; + private BigQueryMetricsEnqueuer metricsEnqueuer; + private DnsQueue dnsQueue; + private DomainFlowTmchUtils domainFlowTmchUtils; + private EppMetric.Builder metricBuilder; + private FakeClock clock; + private ModulesService modulesService; + private Sleeper sleeper; - FakesAndMocksModule() { - this(new FakeClock(), TmchCaMode.PILOT); + public static FakesAndMocksModule create() { + FakeClock clock = new FakeClock(); + return create(clock, TmchCaMode.PILOT, EppMetric.builderForRequest("request-id-1", clock)); } - FakesAndMocksModule(FakeClock clock, TmchCaMode tmchCaMode) { - this.clock = clock; - this.domainFlowTmchUtils = + public static FakesAndMocksModule create( + FakeClock clock, TmchCaMode tmchCaMode, EppMetric.Builder eppMetricBuilder) { + FakesAndMocksModule instance = new FakesAndMocksModule(); + instance.clock = clock; + instance.domainFlowTmchUtils = new DomainFlowTmchUtils(new TmchXmlSignature(new TmchCertificateAuthority(tmchCaMode))); - this.sleeper = new FakeSleeper(clock); - this.dnsQueue = DnsQueue.create(); - this.metricBuilder = EppMetric.builderForRequest("request-id-1", clock); - this.modulesService = mock(ModulesService.class); - this.metricsEnqueuer = mock(BigQueryMetricsEnqueuer.class); + instance.sleeper = new FakeSleeper(clock); + instance.dnsQueue = DnsQueue.create(); + instance.metricBuilder = eppMetricBuilder; + instance.modulesService = mock(ModulesService.class); + instance.metricsEnqueuer = mock(BigQueryMetricsEnqueuer.class); + return instance; } @Provides diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index 99ac8012b..12e0858f3 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -288,11 +288,14 @@ public abstract class FlowTestCase extends ShardableTestCase { .isEqualTo(new TypeInstantiator(getClass()){}.getExactType()); // Run the flow. return DaggerEppTestComponent.builder() - .fakesAndMocksModule(new FakesAndMocksModule(clock, tmchCaMode)) + .fakesAndMocksModule( + FakesAndMocksModule.create( + clock, tmchCaMode, EppMetric.builderForRequest("request-id-1", clock))) .build() .startRequest() .flowComponentBuilder() - .flowModule(new FlowModule.Builder() + .flowModule( + new FlowModule.Builder() .setSessionMetadata(sessionMetadata) .setCredentials(credentials) .setEppRequestSource(eppRequestSource) @@ -301,9 +304,9 @@ public abstract class FlowTestCase extends ShardableTestCase { .setInputXmlBytes(eppLoader.getEppXml().getBytes(UTF_8)) .setEppInput(eppLoader.getEpp()) .build()) - .build() - .flowRunner() - .run(eppMetricBuilder); + .build() + .flowRunner() + .run(eppMetricBuilder); } /** Run a flow and marshal the result to EPP, or throw if it doesn't validate. */