From 42511cfd8b7c56c176c819311ea4dd4ade4df960 Mon Sep 17 00:00:00 2001
From: Makarand Dharmapurikar <makarandd@google.com>
Date: Tue, 27 Sep 2016 18:15:54 -0700
Subject: [PATCH] Addressed review feedback
1. modified documentation
2. changed test slightly to make it more robust to accidental cache hits
---
doc/interop-test-descriptions.md | 27 +++++++++++++++++++--------
test/cpp/interop/interop_client.cc | 9 +++++----
test/cpp/interop/interop_server.cc | 2 +-
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/doc/interop-test-descriptions.md b/doc/interop-test-descriptions.md
index 5b3ad2335c..e3a41b1295 100644
--- a/doc/interop-test-descriptions.md
+++ b/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
diff --git a/test/cpp/interop/interop_client.cc b/test/cpp/interop/interop_client.cc
index f2290adfc3..49ecf2620e 100644
--- a/test/cpp/interop/interop_client.cc
+++ b/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
diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc
index ac2567eba0..64eec4241a 100644
--- a/test/cpp/interop/interop_server.cc
+++ b/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;
}
--
GitLab