Remove the use of InjectRule in UrlFetchUtilsTest

Random used to be a static variable which requires InjectRule to mock it in unit tests. It is now a singleton, which ensures that the same instance is called every time and Random.nextBytes() generates results that distribute uniformly between each call.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=217592767
This commit is contained in:
jianglai 2018-10-17 14:53:23 -07:00
parent 84c3544097
commit 4140ef6315
5 changed files with 35 additions and 12 deletions

View file

@ -14,6 +14,7 @@
package google.registry.config; package google.registry.config;
import static com.google.common.base.Randoms.insecureRandom;
import static com.google.common.base.Suppliers.memoize; import static com.google.common.base.Suppliers.memoize;
import static google.registry.config.ConfigUtils.makeUrl; import static google.registry.config.ConfigUtils.makeUrl;
import static java.lang.annotation.RetentionPolicy.RUNTIME; import static java.lang.annotation.RetentionPolicy.RUNTIME;
@ -34,6 +35,7 @@ import java.lang.annotation.Retention;
import java.net.URI; import java.net.URI;
import java.net.URL; import java.net.URL;
import java.util.Optional; import java.util.Optional;
import java.util.Random;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import javax.inject.Named; import javax.inject.Named;
@ -1257,6 +1259,17 @@ public final class RegistryConfig {
.build()) .build())
.build(); .build();
} }
/**
* Returns a singleton random number generator.
*
* @see google.registry.util.UrlFetchUtils
*/
@Singleton
@Provides
public static Random provideRandom() {
return insecureRandom();
}
} }
/** /**

View file

@ -80,6 +80,7 @@ public final class NordnUploadAction implements Runnable {
private final String actionLogId = String.valueOf(1000000000 + new Random().nextInt(1000000000)); private final String actionLogId = String.valueOf(1000000000 + new Random().nextInt(1000000000));
@Inject Clock clock; @Inject Clock clock;
@Inject Random random;
@Inject LordnRequestInitializer lordnRequestInitializer; @Inject LordnRequestInitializer lordnRequestInitializer;
@Inject URLFetchService fetchService; @Inject URLFetchService fetchService;
@Inject @Config("tmchMarksdbUrl") String tmchMarksdbUrl; @Inject @Config("tmchMarksdbUrl") String tmchMarksdbUrl;
@ -138,7 +139,7 @@ public final class NordnUploadAction implements Runnable {
"LORDN upload task %s: Sending to URL: %s ; data: %s", actionLogId, url, csvData); "LORDN upload task %s: Sending to URL: %s ; data: %s", actionLogId, url, csvData);
HTTPRequest req = new HTTPRequest(new URL(url), POST, validateCertificate().setDeadline(60d)); HTTPRequest req = new HTTPRequest(new URL(url), POST, validateCertificate().setDeadline(60d));
lordnRequestInitializer.initialize(req, tld); lordnRequestInitializer.initialize(req, tld);
setPayloadMultipart(req, "file", "claims.csv", CSV_UTF_8, csvData); setPayloadMultipart(req, "file", "claims.csv", CSV_UTF_8, csvData, random);
HTTPResponse rsp = fetchService.fetch(req); HTTPResponse rsp = fetchService.fetch(req);
logger.atInfo().log( logger.atInfo().log(
"LORDN upload task %s response: HTTP response code %d, response data: %s", "LORDN upload task %s response: HTTP response code %d, response data: %s",

View file

@ -35,9 +35,6 @@ import java.util.Random;
/** Helper methods for the App Engine URL fetch service. */ /** Helper methods for the App Engine URL fetch service. */
public final class UrlFetchUtils { public final class UrlFetchUtils {
@NonFinalForTesting
private static Random random = new Random();
/** Returns value of first header matching {@code name}. */ /** Returns value of first header matching {@code name}. */
public static Optional<String> getHeaderFirst(HTTPResponse rsp, String name) { public static Optional<String> getHeaderFirst(HTTPResponse rsp, String name) {
return getHeaderFirstInternal(rsp.getHeadersUncombined(), name); return getHeaderFirstInternal(rsp.getHeadersUncombined(), name);
@ -63,11 +60,16 @@ public final class UrlFetchUtils {
* *
* <p>This is equivalent to running the command: {@code curl -F fieldName=@payload.txt URL} * <p>This is equivalent to running the command: {@code curl -F fieldName=@payload.txt URL}
* *
* @see <a href="http://www.ietf.org/rfc/rfc2388.txt"> RFC2388 - Returning Values from Forms</a> * @see <a href="http://www.ietf.org/rfc/rfc2388.txt">RFC2388 - Returning Values from Forms</a>
*/ */
public static void setPayloadMultipart( public static void setPayloadMultipart(
HTTPRequest request, String name, String filename, MediaType contentType, String data) { HTTPRequest request,
String boundary = createMultipartBoundary(); String name,
String filename,
MediaType contentType,
String data,
Random random) {
String boundary = createMultipartBoundary(random);
checkState( checkState(
!data.contains(boundary), !data.contains(boundary),
"Multipart data contains autogenerated boundary: %s", boundary); "Multipart data contains autogenerated boundary: %s", boundary);
@ -87,7 +89,7 @@ public final class UrlFetchUtils {
request.setPayload(payload); request.setPayload(payload);
} }
private static String createMultipartBoundary() { private static String createMultipartBoundary(Random random) {
// Generate 192 random bits (24 bytes) to produce 192/log_2(64) = 192/6 = 32 base64 digits. // Generate 192 random bits (24 bytes) to produce 192/log_2(64) = 192/6 = 32 base64 digits.
byte[] rand = new byte[24]; byte[] rand = new byte[24];
random.nextBytes(rand); random.nextBytes(rand);

View file

@ -14,6 +14,7 @@
package google.registry.tmch; package google.registry.tmch;
import static com.google.common.base.Randoms.insecureRandom;
import static com.google.common.net.HttpHeaders.AUTHORIZATION; import static com.google.common.net.HttpHeaders.AUTHORIZATION;
import static com.google.common.net.HttpHeaders.CONTENT_TYPE; import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
import static com.google.common.net.HttpHeaders.LOCATION; import static com.google.common.net.HttpHeaders.LOCATION;
@ -115,6 +116,7 @@ public class NordnUploadActionTest {
action.taskQueueUtils = new TaskQueueUtils(new Retrier(new FakeSleeper(clock), 3)); action.taskQueueUtils = new TaskQueueUtils(new Retrier(new FakeSleeper(clock), 3));
action.tld = "tld"; action.tld = "tld";
action.tmchMarksdbUrl = "http://127.0.0.1"; action.tmchMarksdbUrl = "http://127.0.0.1";
action.random = insecureRandom();
} }
@Test @Test

View file

@ -52,10 +52,10 @@ public class UrlFetchUtilsTest {
@Rule @Rule
public final InjectRule inject = new InjectRule(); public final InjectRule inject = new InjectRule();
private final Random random = mock(Random.class);
@Before @Before
public void setupRandomZeroes() { public void setupRandomZeroes() {
Random random = mock(Random.class);
inject.setStaticField(UrlFetchUtils.class, "random", random);
doAnswer( doAnswer(
info -> { info -> {
Arrays.fill((byte[]) info.getArguments()[0], (byte) 0); Arrays.fill((byte[]) info.getArguments()[0], (byte) 0);
@ -69,7 +69,12 @@ public class UrlFetchUtilsTest {
public void testSetPayloadMultipart() { public void testSetPayloadMultipart() {
HTTPRequest request = mock(HTTPRequest.class); HTTPRequest request = mock(HTTPRequest.class);
setPayloadMultipart( setPayloadMultipart(
request, "lol", "cat", CSV_UTF_8, "The nice people at the store say hello. ヘ(◕。◕ヘ)"); request,
"lol",
"cat",
CSV_UTF_8,
"The nice people at the store say hello. ヘ(◕。◕ヘ)",
random);
ArgumentCaptor<HTTPHeader> headerCaptor = ArgumentCaptor.forClass(HTTPHeader.class); ArgumentCaptor<HTTPHeader> headerCaptor = ArgumentCaptor.forClass(HTTPHeader.class);
verify(request, times(2)).addHeader(headerCaptor.capture()); verify(request, times(2)).addHeader(headerCaptor.capture());
List<HTTPHeader> addedHeaders = headerCaptor.getAllValues(); List<HTTPHeader> addedHeaders = headerCaptor.getAllValues();
@ -97,7 +102,7 @@ public class UrlFetchUtilsTest {
IllegalStateException thrown = IllegalStateException thrown =
assertThrows( assertThrows(
IllegalStateException.class, IllegalStateException.class,
() -> setPayloadMultipart(request, "lol", "cat", CSV_UTF_8, payload)); () -> setPayloadMultipart(request, "lol", "cat", CSV_UTF_8, payload, random));
assertThat(thrown) assertThat(thrown)
.hasMessageThat() .hasMessageThat()
.contains( .contains(