Addressed review feedback

1. modified documentation
2. changed test slightly to make it more robust to accidental cache hits

doc/interop-test-descriptions.md

@@ -65,21 +65,21 @@ ensure that the proto serialized to zero bytes.*
 This test verifies that gRPC requests marked as cacheable use GET verb instead
 of POST, and that server sets appropriate cache control headers for the response
 to be cached by a proxy. This interop test requires that the server is behind
-a caching proxy. It is NOT expected to pass if client is accessing the server
-directly.
+a caching proxy. Use of current timestamp in the request prevents accidental
+cache matches left over from previous tests.
 
 Server features:
 * [CacheableUnaryCall][]
 
 Procedure:
- 1. Client calls CacheableUnaryCall twice, and compares the two responses.
- The server generates a unique response (timestamp) for each request.
- If the second response was delivered from cache, then both responses will
- be the same.
+ 1. Client calls CacheableUnaryCall with `SimpleRequest` request with payload
+    set to current timestamp.
+ 2. Client calls CacheableUnaryCall with `SimpleRequest` request again
+    immediately with the same payload as the previous request.
 
 Client asserts:
-* call was successful
-* responses are the same.
+* Both calls were successful
+* The payload body of both responses is the same.
 
 ### large_unary
 
@@ -962,6 +962,17 @@ payload body of size `SimpleRequest.response_size` bytes and type as appropriate
 for the `SimpleRequest.response_type`. If the server does not support the
 `response_type`, then it should fail the RPC with `INVALID_ARGUMENT`.
 
+### CacheableUnaryCall
+
+Server gets the default Empty proto as the request. It returns the
+SimpleResponse proto with the payload set to current timestamp string.
+In addition it adds
+  1. cache control headers such that the response can be cached by proxies in
+     the response path. Server should be behind a caching proxy for this test
+     to pass.
+  2. adds a `x-user-ip` header with `1.2.3.4` to the response. This is done
+     since some proxys such as GFE will not cache requests from localhost.
+
 ### CompressedResponse
 [CompressedResponse]: #compressedresponse
 

test/cpp/interop/interop_client.cc

@@ -848,10 +848,11 @@ bool InteropClient::DoStatusWithMessage() {
 bool InteropClient::DoCacheableUnary() {
   gpr_log(GPR_DEBUG, "Sending RPC with cacheable response");
 
+  // Create request with current timestamp
+  gpr_timespec ts = gpr_now(GPR_CLOCK_REALTIME);
+  std::string timestamp = std::to_string(ts.tv_nsec);
   SimpleRequest request;
-  request.set_response_size(16);
-  grpc::string payload(16, '\0');
-  request.mutable_payload()->set_body(payload.c_str(), 16);
+  request.mutable_payload()->set_body(timestamp.c_str(), timestamp.size());
 
   // Request 1
   ClientContext context1;
@@ -878,7 +879,7 @@ bool InteropClient::DoCacheableUnary() {
   if (!AssertStatusOk(s2)) {
     return false;
   }
-  gpr_log(GPR_DEBUG, "response 1 payload: %s",
+  gpr_log(GPR_DEBUG, "response 2 payload: %s",
           response2.payload().body().c_str());
 
   // Check that the body is same for both requests. It will be the same if the

test/cpp/interop/interop_server.cc

@@ -159,7 +159,7 @@ class TestServiceImpl : public TestService::Service {
     gpr_timespec ts = gpr_now(GPR_CLOCK_REALTIME);
     std::string timestamp = std::to_string(ts.tv_nsec);
     response->mutable_payload()->set_body(timestamp.c_str(), timestamp.size());
-    context->AddInitialMetadata("Cache-Control", "max-age=100000, public");
+    context->AddInitialMetadata("cache-control", "max-age=100000, public");
     return Status::OK;
   }