From 36ba7c3ea5e33a6647da717f93f5153cd1eafbeb Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Tue, 16 Aug 2016 10:34:21 +0200 Subject: [PATCH] Add missing field to Logging's HttpRequest class (#1160) --- .../com/google/cloud/logging/HttpRequest.java | 143 +++++++----------- .../google/cloud/logging/HttpRequestTest.java | 81 ++++------ 2 files changed, 83 insertions(+), 141 deletions(-) diff --git a/gcloud-java-logging/src/main/java/com/google/cloud/logging/HttpRequest.java b/gcloud-java-logging/src/main/java/com/google/cloud/logging/HttpRequest.java index 7b877f6de1da..ebe14032933d 100644 --- a/gcloud-java-logging/src/main/java/com/google/cloud/logging/HttpRequest.java +++ b/gcloud-java-logging/src/main/java/com/google/cloud/logging/HttpRequest.java @@ -40,15 +40,12 @@ public final class HttpRequest implements Serializable { private final Long responseSize; private final String userAgent; private final String remoteIp; - // todo(mziccard) uncomment when #1042 gets fixed - // private final String serverIp; + private final String serverIp; private final String referer; - // todo(mziccard) uncomment when #1042 gets fixed - // private final boolean cacheLookup; + private final boolean cacheLookup; private final boolean cacheHit; private final boolean cacheValidatedWithOriginServer; - // todo(mziccard) uncomment when #1042 gets fixed - // private final Long cacheFillBytes; + private final Long cacheFillBytes; /** * The HTTP request method. @@ -72,15 +69,12 @@ public static final class Builder { private Long responseSize; private String userAgent; private String remoteIp; - // todo(mziccard) uncomment when #1042 gets fixed - // private String serverIp; + private String serverIp; private String referer; - // todo(mziccard) uncomment when #1042 gets fixed - // private boolean cacheLookup; + private boolean cacheLookup; private boolean cacheHit; private boolean cacheValidatedWithOriginServer; - // todo(mziccard) uncomment when #1042 gets fixed - // private Long cacheFillBytes; + private Long cacheFillBytes; Builder() {} @@ -92,15 +86,12 @@ public static final class Builder { this.responseSize = request.responseSize; this.userAgent = request.userAgent; this.remoteIp = request.remoteIp; - // todo(mziccard) uncomment when #1042 gets fixed - // this.serverIp = request.serverIp; + this.serverIp = request.serverIp; this.referer = request.referer; - // todo(mziccard) uncomment when #1042 gets fixed - // this.cacheLookup = request.cacheLookup; + this.cacheLookup = request.cacheLookup; this.cacheHit = request.cacheHit; this.cacheValidatedWithOriginServer = request.cacheValidatedWithOriginServer; - // todo(mziccard) uncomment when #1042 gets fixed - // this.cacheFillBytes = request.cacheFillBytes; + this.cacheFillBytes = request.cacheFillBytes; } /** @@ -169,11 +160,10 @@ public Builder remoteIp(String remoteIp) { * Sets the IP address (IPv4 or IPv6) of the origin server that the request was sent to. * Examples: {@code 192.168.1.1}, {@code FE80::0202:B3FF:FE1E:8329}. */ - // todo(mziccard) uncomment when #1042 gets fixed - //public Builder serverIp(String serverIp) { - // this.serverIp = serverIp; - // return this; - //} + public Builder serverIp(String serverIp) { + this.serverIp = serverIp; + return this; + } /** * Sets the referer URL of the request, as defined in HTTP/1.1 Header Field Definitions. @@ -189,11 +179,10 @@ public Builder referer(String referer) { /** * Sets whether or not a cache lookup was attempted. If not set, {@code false} is used. */ - // todo(mziccard) uncomment when #1042 gets fixed - //public Builder cacheLookup(boolean cacheLookup) { - // this.cacheLookup = cacheLookup; - // return this; - //} + public Builder cacheLookup(boolean cacheLookup) { + this.cacheLookup = cacheLookup; + return this; + } /** * Sets whether or not an entity was served from cache (with or without validation). If not set, @@ -218,11 +207,10 @@ public Builder cacheValidatedWithOriginServer(boolean cacheValidatedWithOriginSe * Sets the number of HTTP response bytes inserted into cache. Set only when a cache fill was * attempted. */ - // todo(mziccard) uncomment when #1042 gets fixed - //public Builder cacheFillBytes(long cacheFillBytes) { - // this.cacheFillBytes = cacheFillBytes; - // return this; - //} + public Builder cacheFillBytes(long cacheFillBytes) { + this.cacheFillBytes = cacheFillBytes; + return this; + } /** * Creates a {@code HttpRequest} object for this builder. @@ -240,15 +228,12 @@ public HttpRequest build() { this.responseSize = builder.responseSize; this.userAgent = builder.userAgent; this.remoteIp = builder.remoteIp; - // todo(mziccard) uncomment when #1042 gets fixed - // this.serverIp = builder.serverIp; + this.serverIp = builder.serverIp; this.referer = builder.referer; - // todo(mziccard) uncomment when #1042 gets fixed - // this.cacheLookup = builder.cacheLookup; + this.cacheLookup = builder.cacheLookup; this.cacheHit = builder.cacheHit; this.cacheValidatedWithOriginServer = builder.cacheValidatedWithOriginServer; - // todo(mziccard) uncomment when #1042 gets fixed - // this.cacheFillBytes = builder.cacheFillBytes; + this.cacheFillBytes = builder.cacheFillBytes; } /** @@ -310,10 +295,9 @@ public String remoteIp() { * Returns the IP address (IPv4 or IPv6) of the origin server that the request was sent to. * Examples: {@code 192.168.1.1}, {@code FE80::0202:B3FF:FE1E:8329}. */ - // todo(mziccard) uncomment when #1042 gets fixed - //public String serverIp() { - // return serverIp; - //} + public String serverIp() { + return serverIp; + } /** * Returns the referer URL of the request, as defined in HTTP/1.1 Header Field Definitions. @@ -329,10 +313,9 @@ public String referer() { * Returns whether or not a cache lookup was attempted. If not set, this method returns * {@code false}. */ - // todo(mziccard) uncomment when #1042 gets fixed - //public boolean cacheLookup() { - // return cacheLookup; - //} + public boolean cacheLookup() { + return cacheLookup; + } /** * Returns whether or not an entity was served from cache (with or without validation). If not @@ -355,17 +338,15 @@ public boolean cacheValidatedWithOriginServer() { * Returns the number of HTTP response bytes inserted into cache. Set only when a cache fill was * attempted. */ - // todo(mziccard) uncomment when #1042 gets fixed - //public Long cacheFillBytes() { - // return cacheFillBytes; - //} + public Long cacheFillBytes() { + return cacheFillBytes; + } @Override public int hashCode() { return Objects.hash(requestMethod, requestUrl, requestSize, status, responseSize, userAgent, - // todo(mziccard) uncomment when #1042 gets fixed - // serverIp, cacheLookup, cacheFillBytes - remoteIp, referer, cacheHit, cacheValidatedWithOriginServer); + serverIp, cacheLookup, cacheFillBytes, remoteIp, referer, cacheHit, + cacheValidatedWithOriginServer); } @Override @@ -378,15 +359,12 @@ public String toString() { .add("responseSize", responseSize) .add("userAgent", userAgent) .add("remoteIp", remoteIp) - // todo(mziccard) uncomment when #1042 gets fixed - // .add("serverIp", serverIp) + .add("serverIp", serverIp) .add("referer", referer) - // todo(mziccard) uncomment when #1042 gets fixed - // .add("cacheLookup", cacheLookup) + .add("cacheLookup", cacheLookup) .add("cacheHit", cacheHit) .add("cacheValidatedWithOriginServer", cacheValidatedWithOriginServer) - // todo(mziccard) uncomment when #1042 gets fixed - // .add("cacheFillBytes", cacheFillBytes) + .add("cacheFillBytes", cacheFillBytes) .toString(); } @@ -406,15 +384,12 @@ public boolean equals(Object obj) { && Objects.equals(responseSize, other.responseSize) && Objects.equals(userAgent, other.userAgent) && Objects.equals(remoteIp, other.remoteIp) - // todo(mziccard) uncomment when #1042 gets fixed - // && Objects.equals(serverIp, other.serverIp) + && Objects.equals(serverIp, other.serverIp) && Objects.equals(referer, other.referer) - // todo(mziccard) uncomment when #1042 gets fixed - // && cacheLookup == other.cacheLookup + && cacheLookup == other.cacheLookup && cacheHit == other.cacheHit - && cacheValidatedWithOriginServer == other.cacheValidatedWithOriginServer; - // todo(mziccard) uncomment when #1042 gets fixed - // && Objects.equals(cacheFillBytes, other.cacheFillBytes); + && cacheValidatedWithOriginServer == other.cacheValidatedWithOriginServer + && Objects.equals(cacheFillBytes, other.cacheFillBytes); } /** @@ -448,21 +423,18 @@ com.google.logging.type.HttpRequest toPb() { if (remoteIp != null) { builder.setRemoteIp(remoteIp); } - // todo(mziccard) uncomment when #1042 gets fixed - // if (serverIp != null) { - // builder.setServerIp(serverIp); - //} + if (serverIp != null) { + builder.setServerIp(serverIp); + } if (referer != null) { builder.setReferer(referer); } - // todo(mziccard) uncomment when #1042 gets fixed - // builder.setCacheLookup(cacheLookup); + builder.setCacheLookup(cacheLookup); builder.setCacheHit(cacheHit); builder.setCacheValidatedWithOriginServer(cacheValidatedWithOriginServer); - // todo(mziccard) uncomment when #1042 gets fixed - //if (cacheFillBytes != null) { - // builder.setCacheFillBytes(cacheFillBytes); - //} + if (cacheFillBytes != null) { + builder.setCacheFillBytes(cacheFillBytes); + } return builder.build(); } @@ -493,24 +465,21 @@ static HttpRequest fromPb(com.google.logging.type.HttpRequest requestPb) { if (requestPb.getUserAgent() != null && !requestPb.getRequestUrl().equals("")) { builder.userAgent(requestPb.getUserAgent()); } - // todo(mziccard) uncomment when #1042 gets fixed - //if (requestPb.getServerIp() != null && !requestPb.getServerIp().equals("")) { - // builder.serverIp(requestPb.getServerIp()); - //} + if (requestPb.getServerIp() != null && !requestPb.getServerIp().equals("")) { + builder.serverIp(requestPb.getServerIp()); + } if (requestPb.getRemoteIp() != null && !requestPb.getRemoteIp().equals("")) { builder.remoteIp(requestPb.getRemoteIp()); } if (requestPb.getReferer() != null && !requestPb.getReferer().equals("")) { builder.referer(requestPb.getReferer()); } - // todo(mziccard) uncomment when #1042 gets fixed - // builder.cacheLookup(requestPb.getCacheLookup()); + builder.cacheLookup(requestPb.getCacheLookup()); builder.cacheHit(requestPb.getCacheHit()); builder.cacheValidatedWithOriginServer(requestPb.getCacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - //if (requestPb.getCacheFillBytes() != 0L) { - // builder.cacheFillBytes(requestPb.getCacheFillBytes()); - //} + if (requestPb.getCacheFillBytes() != 0L) { + builder.cacheFillBytes(requestPb.getCacheFillBytes()); + } return builder.build(); } } diff --git a/gcloud-java-logging/src/test/java/com/google/cloud/logging/HttpRequestTest.java b/gcloud-java-logging/src/test/java/com/google/cloud/logging/HttpRequestTest.java index dc55de92c72c..41eba99b327a 100644 --- a/gcloud-java-logging/src/test/java/com/google/cloud/logging/HttpRequestTest.java +++ b/gcloud-java-logging/src/test/java/com/google/cloud/logging/HttpRequestTest.java @@ -37,15 +37,12 @@ public class HttpRequestTest { private static final String USER_AGENT = "Mozilla/4.0 (compatible; MSIE 6.0; Windows 98; Q312461; .NET CLR 1.0.3705)"; private static final String REMOTE_IP = "192.168.1.1"; - // todo(mziccard) uncomment when #1042 gets fixed - // private static final String SERVER_IP = "192.168.1.2"; + private static final String SERVER_IP = "192.168.1.2"; private static final String REFERER = "Referer: http://www.example.com"; - // todo(mziccard) uncomment when #1042 gets fixed - // private static final boolean CACHE_LOOKUP = true; + private static final boolean CACHE_LOOKUP = true; private static final boolean CACHE_HIT = true; private static final boolean CACHE_VALIDATED_WITH_ORIGIN_SERVER = false; - // todo(mziccard) uncomment when #1042 gets fixed - // private static final Long CACHE_FILL_BYTES = 3L; + private static final Long CACHE_FILL_BYTES = 3L; private static final HttpRequest HTTP_REQUEST = HttpRequest.builder() .requestMethod(REQUEST_METHOD) .requestUrl(REQUEST_URL) @@ -54,15 +51,12 @@ public class HttpRequestTest { .responseSize(REPONSE_SIZE) .userAgent(USER_AGENT) .remoteIp(REMOTE_IP) - // todo(mziccard) uncomment when #1042 gets fixed - // .serverIp(SERVER_IP) + .serverIp(SERVER_IP) .referer(REFERER) - // todo(mziccard) uncomment when #1042 gets fixed - // .cacheLookup(CACHE_LOOKUP) + .cacheLookup(CACHE_LOOKUP) .cacheHit(CACHE_HIT) .cacheValidatedWithOriginServer(CACHE_VALIDATED_WITH_ORIGIN_SERVER) - // todo(mziccard) uncomment when #1042 gets fixed - // .cacheFillBytes(CACHE_FILL_BYTES) + .cacheFillBytes(CACHE_FILL_BYTES) .build(); @Rule @@ -77,15 +71,12 @@ public void testBuilder() { assertEquals(REPONSE_SIZE, HTTP_REQUEST.responseSize()); assertEquals(USER_AGENT, HTTP_REQUEST.userAgent()); assertEquals(REMOTE_IP, HTTP_REQUEST.remoteIp()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(SERVER_IP, HTTP_REQUEST.serverIp()); + assertEquals(SERVER_IP, HTTP_REQUEST.serverIp()); assertEquals(REFERER, HTTP_REQUEST.referer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(CACHE_LOOKUP, HTTP_REQUEST.cacheLookup()); + assertEquals(CACHE_LOOKUP, HTTP_REQUEST.cacheLookup()); assertEquals(CACHE_HIT, HTTP_REQUEST.cacheHit()); assertEquals(CACHE_VALIDATED_WITH_ORIGIN_SERVER, HTTP_REQUEST.cacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(CACHE_FILL_BYTES, HTTP_REQUEST.cacheFillBytes()); + assertEquals(CACHE_FILL_BYTES, HTTP_REQUEST.cacheFillBytes()); } @Test @@ -98,15 +89,12 @@ public void testBuilderDefaultValues() { assertNull(httpRequest.responseSize()); assertNull(httpRequest.userAgent()); assertNull(httpRequest.remoteIp()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertNull(httpRequest.serverIp()); + assertNull(httpRequest.serverIp()); assertNull(httpRequest.referer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertFalse(httpRequest.cacheLookup()); + assertFalse(httpRequest.cacheLookup()); assertFalse(httpRequest.cacheHit()); assertFalse(httpRequest.cacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertNull(httpRequest.cacheFillBytes()); + assertNull(httpRequest.cacheFillBytes()); } @Test @@ -120,15 +108,12 @@ public void testToBuilder() { .responseSize(5) .userAgent("otherUserAgent") .remoteIp("192.168.1.3") - // todo(mziccard) uncomment when #1042 gets fixed - // .serverIp("192.168.1.4") + .serverIp("192.168.1.4") .referer("Referer: http://www.other-example.com") - // todo(mziccard) uncomment when #1042 gets fixed - // .cacheLookup(true) + .cacheLookup(true) .cacheHit(true) .cacheValidatedWithOriginServer(true) - // todo(mziccard) uncomment when #1042 gets fixed - // .cacheFillBytes(6) + .cacheFillBytes(6) .build(); assertEquals(RequestMethod.POST, httpRequest.requestMethod()); assertEquals("http://www.other-example.com", httpRequest.requestUrl()); @@ -137,15 +122,12 @@ public void testToBuilder() { assertEquals(5, (long) httpRequest.responseSize()); assertEquals("otherUserAgent", httpRequest.userAgent()); assertEquals("192.168.1.3", httpRequest.remoteIp()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals("192.168.1.4", httpRequest.serverIp()); + assertEquals("192.168.1.4", httpRequest.serverIp()); assertEquals("Referer: http://www.other-example.com", httpRequest.referer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertTrue(httpRequest.cacheLookup()); + assertTrue(httpRequest.cacheLookup()); assertTrue(httpRequest.cacheHit()); assertTrue(httpRequest.cacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(6, (long) httpRequest.cacheFillBytes()); + assertEquals(6, (long) httpRequest.cacheFillBytes()); } @Test @@ -159,15 +141,12 @@ public void testToAndFromPb() { assertEquals(REPONSE_SIZE, httpRequest.responseSize()); assertEquals(USER_AGENT, httpRequest.userAgent()); assertEquals(REMOTE_IP, httpRequest.remoteIp()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(SERVER_IP, httpRequest.serverIp()); + assertEquals(SERVER_IP, httpRequest.serverIp()); assertEquals(REFERER, httpRequest.referer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(CACHE_LOOKUP, httpRequest.cacheLookup()); + assertEquals(CACHE_LOOKUP, httpRequest.cacheLookup()); assertEquals(CACHE_HIT, httpRequest.cacheHit()); assertEquals(CACHE_VALIDATED_WITH_ORIGIN_SERVER, httpRequest.cacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(CACHE_FILL_BYTES, httpRequest.cacheFillBytes()); + assertEquals(CACHE_FILL_BYTES, httpRequest.cacheFillBytes()); HttpRequest incompleteHttpRequest = HttpRequest.builder().build(); httpRequest = HttpRequest.fromPb(incompleteHttpRequest.toPb()); compareHttpRequest(incompleteHttpRequest, httpRequest); @@ -178,15 +157,12 @@ public void testToAndFromPb() { assertNull(httpRequest.responseSize()); assertNull(httpRequest.userAgent()); assertNull(httpRequest.remoteIp()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertNull(httpRequest.serverIp()); + assertNull(httpRequest.serverIp()); assertNull(httpRequest.referer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertFalse(httpRequest.cacheLookup()); + assertFalse(httpRequest.cacheLookup()); assertFalse(httpRequest.cacheHit()); assertFalse(httpRequest.cacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertNull(httpRequest.cacheFillBytes()); + assertNull(httpRequest.cacheFillBytes()); } private void compareHttpRequest(HttpRequest expected, HttpRequest value) { @@ -198,15 +174,12 @@ private void compareHttpRequest(HttpRequest expected, HttpRequest value) { assertEquals(expected.responseSize(), value.responseSize()); assertEquals(expected.userAgent(), value.userAgent()); assertEquals(expected.remoteIp(), value.remoteIp()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(expected.serverIp(), value.serverIp(); + assertEquals(expected.serverIp(), value.serverIp()); assertEquals(expected.referer(), value.referer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(expected.cacheLookup(), value.cacheLookup(); + assertEquals(expected.cacheLookup(), value.cacheLookup()); assertEquals(expected.cacheHit(), value.cacheHit()); assertEquals(expected.cacheValidatedWithOriginServer(), value.cacheValidatedWithOriginServer()); - // todo(mziccard) uncomment when #1042 gets fixed - // assertEquals(expected.cacheFillBytes(), value.cacheFillBytes(); + assertEquals(expected.cacheFillBytes(), value.cacheFillBytes()); assertEquals(expected.hashCode(), value.hashCode()); assertEquals(expected.toString(), value.toString()); }