Merge pull request #419 from uber/lu.sweeptodo

Sweep todos in runtime dir

examples/example-gateway/config/production.json

@@ -16,6 +16,7 @@
 	"clients.corge.serviceName": "Corge",
 	"clients.corge.timeout": 10000,
 	"clients.corge.timeoutPerAttempt": 2000,
+	"clients.corge-http.serviceName": "CorgeHttp",
 	"clients.google-now.ip": "127.0.0.1",
 	"clients.google-now.port": 14120,
 	"clients.google-now.timeout": 10000,

runtime/client_http_request.go

@@ -79,7 +79,7 @@ func (req *ClientHTTPRequest) start() {
 	req.startTime = time.Now()
 }
 
-// CheckHeaders verifies that the outbound request contaisn required headers
+// CheckHeaders verifies that the outbound request contains required headers
 func (req *ClientHTTPRequest) CheckHeaders(expected []string) error {
 	if req.httpReq == nil {
 		/* coverage ignore next line */
@@ -89,7 +89,7 @@ func (req *ClientHTTPRequest) CheckHeaders(expected []string) error {
 	actualHeaders := req.httpReq.Header
 
 	for _, headerName := range expected {
-		// TODO: case sensitivity ?
+		// headerName is case insensitive, http.Header Get canonicalize the key
 		headerValue := actualHeaders.Get(headerName)
 		if headerValue == "" {
 			req.Logger.Warn("Got outbound request without mandatory header",

runtime/client_http_response.go

@@ -170,13 +170,6 @@ func clientHTTPLogFields(req *ClientHTTPRequest, res *ClientHTTPResponse) []zapc
 		zap.Time("timestamp-started", req.startTime),
 		zap.Time("timestamp-finished", res.finishTime),
 		zap.Int("statusCode", res.StatusCode),
-
-		// TODO: Do not log body by default because PII and bandwidth.
-		// Temporarily log during the development cycle
-		// TODO: Add a gateway level configurable body unmarshaller
-		// to extract only non-PII info.
-		zap.ByteString("Request Body", req.rawBody),
-		zap.ByteString("Response Body", res.rawResponseBytes),
 	}
 
 	for k, v := range req.httpReq.Header {

runtime/middlewares_test.go

@@ -56,9 +56,6 @@ func TestHandlers(t *testing.T) {
 	assert.Equal(t, 1, len(middlewares))
 
 	// Run the zanzibar.HandleFn of composed middlewares.
-	// TODO(sindelar): Refactor. We some helpers to build zanzibar
-	// request/responses without setting up a backend and register.
-	// Currently they require endpoints to instantiate.
 	gateway, err := benchGateway.CreateGateway(
 		defaultTestConfig,
 		defaultTestOptions,
@@ -261,9 +258,6 @@ func TestMiddlewareSharedStates(t *testing.T) {
 	assert.Equal(t, 2, len(middlewares))
 
 	// Run the zanzibar.HandleFn of composed middlewares.
-	// TODO(sindelar): Refactor. We some helpers to build zanzibar
-	// request/responses without setting up a backend and register.
-	// Currently they require endpoints to instantiate.
 	gateway, err := benchGateway.CreateGateway(
 		defaultTestConfig,
 		defaultTestOptions,

runtime/router.go

@@ -202,7 +202,6 @@ func (router *HTTPRouter) handleNotFound(
 	r *http.Request,
 ) {
 	req := NewServerHTTPRequest(w, r, nil, router.notFoundEndpoint)
-	// TODO custom NotFound
 	http.NotFound(w, r)
 	req.res.StatusCode = http.StatusNotFound
 	req.res.finish()
@@ -213,8 +212,6 @@ func (router *HTTPRouter) handleMethodNotAllowed(
 	r *http.Request,
 ) {
 	req := NewServerHTTPRequest(w, r, nil, router.methodNotAllowedEndpoint)
-	// TODO: Remove coverage ignore when body unmarshaling supported.
-	// TODO custom MethodNotAllowed
 	http.Error(w,
 		http.StatusText(http.StatusMethodNotAllowed),
 		http.StatusMethodNotAllowed,

runtime/server_header.go

@@ -125,6 +125,9 @@ func (zh ServerHTTPHeader) Ensure(keys []string, logger *zap.Logger) error {
 
 // ServerTChannelHeader wrapper to implement zanzibar Header interface
 // on map[string]string
+// Unlike http.Header, tchannel headers are case sensitive and should be
+// keyed with lower case. TChannel protocol does not mention header case
+// sensitivity, so it is up to implementation.
 type ServerTChannelHeader map[string]string
 
 // Get retrieves the string value stored on a given header. Bool
@@ -142,8 +145,6 @@ func (th ServerTChannelHeader) Add(key string, value string) {
 
 // Set sets a value for a given header, overwriting the previous value.
 func (th ServerTChannelHeader) Set(key string, value string) {
-	// TODO: Canonicalize strings before inserting
-	// Use textproto.CanonicalMIMEHeaderKey
 	th[key] = value
 }
 

runtime/server_header_test.go

@@ -168,23 +168,29 @@ func TestSTHAdd(t *testing.T) {
 	zh := zanzibar.ServerTChannelHeader{}
 
 	zh.Add("foo", "headOne")
-	assert.Equal(t, "headOne", zh["foo"])
+	v, ok := zh.Get("foo")
+	assert.True(t, ok)
+	assert.Equal(t, "headOne", v)
 }
 
 func TestSTHSetOverwriteOldKey(t *testing.T) {
 	zh := zanzibar.ServerTChannelHeader{}
 	zh.Set("foo", "headOne")
 
-	zh.Set("foo", "newHeader")
-	assert.Equal(t, "newHeader", zh["foo"])
+	zh.Add("foo", "newHeader")
+	v, ok := zh.Get("foo")
+	assert.True(t, ok)
+	assert.Equal(t, "newHeader", v)
 }
 
 func TestSTHSetNewKey(t *testing.T) {
 	zh := zanzibar.ServerTChannelHeader{}
 	zh.Set("bar", "otherHeader")
 
-	zh.Set("foo", "headOne")
-	assert.Equal(t, "headOne", zh["foo"])
+	zh.Add("foo", "headOne")
+	v, ok := zh.Get("foo")
+	assert.True(t, ok)
+	assert.Equal(t, "headOne", v)
 }
 
 func TestSTHMissingKey(t *testing.T) {

runtime/server_http_response.go

@@ -120,13 +120,6 @@ func serverHTTPLogFields(req *ServerHTTPRequest, res *ServerHTTPResponse) []zapc
 		zap.Time("timestamp-started", req.startTime),
 		zap.Time("timestamp-finished", res.finishTime),
 		zap.Int("statusCode", res.StatusCode),
-
-		// TODO: Do not log body by default because PII and bandwidth.
-		// Temporarily log during the development cycle
-		// TODO: Add a gateway level configurable body unmarshaller
-		// to extract only non-PII info.
-		// zap.ByteString("Request Body", req.rawBody),
-		// zap.ByteString("Response Body", res.pendingBodyBytes),
 	}
 
 	if res.err != nil {
@@ -149,8 +142,6 @@ func serverHTTPLogFields(req *ServerHTTPRequest, res *ServerHTTPResponse) []zapc
 		}
 	}
 
-	// TODO: log jaeger trace span
-
 	return fields
 }
 
@@ -186,7 +177,6 @@ func (res *ServerHTTPResponse) WriteJSONBytes(
 		}
 	}
 
-	// TODO: mark header as pending ?
 	res.responseWriter.Header().Set("content-type", "application/json")
 
 	res.pendingStatusCode = statusCode
@@ -219,7 +209,6 @@ func (res *ServerHTTPResponse) WriteJSON(
 		}
 	}
 
-	// TODO: mark header as pending ?
 	res.responseWriter.Header().
 		Set("content-type", "application/json")
 

runtime/tchannel_client.go

@@ -300,8 +300,6 @@ func (c *tchannelOutboundCall) logFields() []zapcore.Field {
 		fields = append(fields, zap.String("Response-Header-"+k, v))
 	}
 
-	// TODO: log jaeger trace span
-
 	return fields
 }
 

runtime/tchannel_server.go

@@ -262,8 +262,6 @@ func (c *tchannelInboundCall) logFields() []zapcore.Field {
 		fields = append(fields, zap.String("Response-Header-"+k, v))
 	}
 
-	// TODO: log jaeger trace span
-
 	return fields
 }