Skip to content

Commit

Permalink
Move json and proto marshalling into util/protoutil package.
Browse files Browse the repository at this point in the history 
Also move util/http.go to new util/httputil package.
This makes it much easier for downstream users to use the cockroachdb workaround for inconsistensies between grpc-gateway and gogo/protobuf.
See cockroachdb#10294 and gogo/protobuf#178 for more information.
  • Loading branch information
Johan Brandhorst committed Oct 31, 2016
1 parent be39a8c commit 5769d35
Show file tree
Hide file tree
Showing 19 changed files with 64 additions and 52 deletions.
10 changes: 5 additions & 5 deletions pkg/acceptance/admin_test.go
View file
Expand Up @@ -26,7 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/server/status"
"github.com/cockroachdb/cockroach/pkg/ts/tspb"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
)

Expand All @@ -44,7 +44,7 @@ func testAdminLossOfQuorumInner(t *testing.T, c cluster.Cluster, cfg cluster.Tes
nodeIDs := make([]roachpb.NodeID, c.NumNodes())
for i := 0; i < c.NumNodes(); i++ {
var details serverpb.DetailsResponse
if err := util.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/details/local", &details); err != nil {
if err := httputil.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/details/local", &details); err != nil {
t.Fatal(err)
}
nodeIDs[i] = details.NodeID
Expand All @@ -59,13 +59,13 @@ func testAdminLossOfQuorumInner(t *testing.T, c cluster.Cluster, cfg cluster.Tes

// Retrieve node statuses.
var nodes serverpb.NodesResponse
if err := util.GetJSON(cluster.HTTPClient, c.URL(0)+"/_status/nodes", &nodes); err != nil {
if err := httputil.GetJSON(cluster.HTTPClient, c.URL(0)+"/_status/nodes", &nodes); err != nil {
t.Fatal(err)
}

for _, nodeID := range nodeIDs {
var nodeStatus status.NodeStatus
if err := util.GetJSON(cluster.HTTPClient, c.URL(0)+"/_status/nodes/"+strconv.Itoa(int(nodeID)), &nodeStatus); err != nil {
if err := httputil.GetJSON(cluster.HTTPClient, c.URL(0)+"/_status/nodes/"+strconv.Itoa(int(nodeID)), &nodeStatus); err != nil {
t.Fatal(err)
}
}
Expand All @@ -80,7 +80,7 @@ func testAdminLossOfQuorumInner(t *testing.T, c cluster.Cluster, cfg cluster.Tes
},
}
var queryResponse tspb.TimeSeriesQueryResponse
if err := util.PostJSON(cluster.HTTPClient, c.URL(0)+"/ts/query",
if err := httputil.PostJSON(cluster.HTTPClient, c.URL(0)+"/ts/query",
&queryRequest, &queryResponse); err != nil {
t.Fatal(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/acceptance/allocator_test.go
View file
Expand Up @@ -87,7 +87,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
)
Expand Down Expand Up @@ -204,7 +204,7 @@ func (at *allocatorTest) stdDev() (float64, error) {
var client http.Client
var nodesResp serverpb.NodesResponse
url := fmt.Sprintf("http://%s:%s/_status/nodes", host, adminPort)
if err := util.GetJSON(client, url, &nodesResp); err != nil {
if err := httputil.GetJSON(client, url, &nodesResp); err != nil {
return 0, err
}
var replicaCounts stats.Float64Data
Expand Down
3 changes: 2 additions & 1 deletion pkg/acceptance/build_info_test.go
View file
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/acceptance/cluster"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
)

func TestBuildInfo(t *testing.T) {
Expand All @@ -39,7 +40,7 @@ func testBuildInfoInner(t *testing.T, c cluster.Cluster, cfg cluster.TestConfig)
t.Fatalf("interrupted")
default:
}
return util.GetJSON(cluster.HTTPClient, c.URL(0)+"/_status/details/local", &details)
return httputil.GetJSON(cluster.HTTPClient, c.URL(0)+"/_status/details/local", &details)
})

bi := details.BuildInfo
Expand Down
3 changes: 2 additions & 1 deletion pkg/acceptance/continuous_load_test.go
View file
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/acceptance/terrafarm"
"github.com/cockroachdb/cockroach/pkg/server/status"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
)
Expand Down Expand Up @@ -74,7 +75,7 @@ func (cl continuousLoadTest) queryCount(f *terrafarm.Farmer) (float64, error) {
var client http.Client
var resp status.NodeStatus
host := f.Nodes()[0]
if err := util.GetJSON(client, "http://"+host+":8080/_status/nodes/local", &resp); err != nil {
if err := httputil.GetJSON(client, "http://"+host+":8080/_status/nodes/local", &resp); err != nil {
return 0, err
}
count, ok := resp.Metrics["sql.query.count"]
Expand Down
3 changes: 2 additions & 1 deletion pkg/acceptance/freeze_test.go
View file
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
)

Expand All @@ -59,7 +60,7 @@ func postFreeze(
log.Infof(context.Background(), "%+v", &resp)
}
}
err := util.StreamJSON(
err := httputil.StreamJSON(
httpClient,
c.URL(0)+"/_admin/v1/cluster/freeze",
&serverpb.ClusterFreezeRequest{Freeze: freeze},
Expand Down
3 changes: 2 additions & 1 deletion pkg/acceptance/gossip_peerings_test.go
View file
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/gossip"
"github.com/cockroachdb/cockroach/pkg/internal/client"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
)
Expand All @@ -53,7 +54,7 @@ func checkGossip(t *testing.T, c cluster.Cluster, d time.Duration, f checkGossip

var infoStatus gossip.InfoStatus
for i := 0; i < c.NumNodes(); i++ {
if err := util.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/gossip/local", &infoStatus); err != nil {
if err := httputil.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/gossip/local", &infoStatus); err != nil {
return err
}
if err := f(infoStatus.Infos); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/acceptance/status_server_test.go
View file
Expand Up @@ -30,7 +30,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/acceptance/cluster"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/retry"
)
Expand Down Expand Up @@ -82,7 +82,7 @@ func checkNode(
}
var details serverpb.DetailsResponse
for _, urlID := range urlIDs {
if err := util.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/details/"+urlID, &details); err != nil {
if err := httputil.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/details/"+urlID, &details); err != nil {
t.Fatal(errors.Errorf("unable to parse details - %s", err))
}
if details.NodeID != expectedNodeID {
Expand All @@ -108,7 +108,7 @@ func testStatusServerInner(t *testing.T, c cluster.Cluster, cfg cluster.TestConf
idMap := make(map[int]roachpb.NodeID)
for i := 0; i < c.NumNodes(); i++ {
var details serverpb.DetailsResponse
if err := util.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/details/local", &details); err != nil {
if err := httputil.GetJSON(cluster.HTTPClient, c.URL(i)+"/_status/details/local", &details); err != nil {
t.Fatal(err)
}
idMap[i] = details.NodeID
Expand Down
7 changes: 4 additions & 3 deletions pkg/server/admin_cluster_test.go
View file
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
)

Expand Down Expand Up @@ -84,7 +85,7 @@ func TestAdminAPITableStats(t *testing.T) {
// The new SQL table may not yet have split into its own range. Wait for
// this to occur, and for full replication.
util.SucceedsSoon(t, func() error {
if err := util.GetJSON(client, url, &tsResponse); err != nil {
if err := httputil.GetJSON(client, url, &tsResponse); err != nil {
return err
}
if tsResponse.RangeCount != 1 {
Expand Down Expand Up @@ -112,7 +113,7 @@ func TestAdminAPITableStats(t *testing.T) {
// lower.
tc.StopServer(1)

if err := util.GetJSON(client, url, &tsResponse); err != nil {
if err := httputil.GetJSON(client, url, &tsResponse); err != nil {
t.Fatal(err)
}
if a, e := tsResponse.NodeCount, int64(nodeCount); a != e {
Expand All @@ -137,5 +138,5 @@ func TestAdminAPITableStats(t *testing.T) {
// timeout; however, in aggregate (or in stress tests) this will suffice for
// detecting leaks.
client.Timeout = 1 * time.Nanosecond
_ = util.GetJSON(client, url, &tsResponse)
_ = httputil.GetJSON(client, url, &tsResponse)
}
5 changes: 3 additions & 2 deletions pkg/server/admin_test.go
View file
Expand Up @@ -45,6 +45,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -981,14 +982,14 @@ func TestClusterFreeze(t *testing.T) {
}
path := s.AdminURL() + adminPrefix + "cluster/freeze"

if err := util.StreamJSON(cli, path, &req, &serverpb.ClusterFreezeResponse{}, cb); err != nil {
if err := httputil.StreamJSON(cli, path, &req, &serverpb.ClusterFreezeResponse{}, cb); err != nil {
t.Fatal(err)
}
if aff := resp.RangesAffected; aff == 0 {
t.Fatalf("expected affected ranges: %+v", resp)
}

if err := util.StreamJSON(cli, path, &req, &serverpb.ClusterFreezeResponse{}, cb); err != nil {
if err := httputil.StreamJSON(cli, path, &req, &serverpb.ClusterFreezeResponse{}, cb); err != nil {
t.Fatal(err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/authentication_test.go
View file
Expand Up @@ -30,7 +30,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/ts"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/gogo/protobuf/proto"
Expand All @@ -52,7 +52,7 @@ func doHTTPReq(
t.Fatalf("%s %s: error building request: %s", method, url, err)
}
if b != nil {
req.Header.Add(util.ContentTypeHeader, util.ProtoContentType)
req.Header.Add(httputil.ContentTypeHeader, httputil.ProtoContentType)
}

return client.Do(req)
Expand Down
20 changes: 11 additions & 9 deletions pkg/server/server.go
View file
Expand Up @@ -55,9 +55,11 @@ import (
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/metric"
"github.com/cockroachdb/cockroach/pkg/util/netutil"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/sdnotify"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -620,18 +622,18 @@ func (s *Server) Start(ctx context.Context) error {
log.Event(ctx, "accepting connections")

// Initialize grpc-gateway mux and context.
jsonpb := &util.JSONPb{
jsonpb := &protoutil.JSONPb{
EnumsAsInts: true,
EmitDefaults: true,
Indent: " ",
}
protopb := new(util.ProtoPb)
protopb := new(protoutil.ProtoPb)
gwMux := gwruntime.NewServeMux(
gwruntime.WithMarshalerOption(gwruntime.MIMEWildcard, jsonpb),
gwruntime.WithMarshalerOption(util.JSONContentType, jsonpb),
gwruntime.WithMarshalerOption(util.AltJSONContentType, jsonpb),
gwruntime.WithMarshalerOption(util.ProtoContentType, protopb),
gwruntime.WithMarshalerOption(util.AltProtoContentType, protopb),
gwruntime.WithMarshalerOption(httputil.JSONContentType, jsonpb),
gwruntime.WithMarshalerOption(httputil.AltJSONContentType, jsonpb),
gwruntime.WithMarshalerOption(httputil.ProtoContentType, protopb),
gwruntime.WithMarshalerOption(httputil.AltProtoContentType, protopb),
)
gwCtx, gwCancel := context.WithCancel(s.AnnotateCtx(context.Background()))
s.stopper.AddCloser(stop.CloserFn(gwCancel))
Expand Down Expand Up @@ -767,10 +769,10 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// Disable caching of responses.
w.Header().Set("Cache-control", "no-cache")

ae := r.Header.Get(util.AcceptEncodingHeader)
ae := r.Header.Get(httputil.AcceptEncodingHeader)
switch {
case strings.Contains(ae, util.GzipEncoding):
w.Header().Set(util.ContentEncodingHeader, util.GzipEncoding)
case strings.Contains(ae, httputil.GzipEncoding):
w.Header().Set(httputil.ContentEncodingHeader, httputil.GzipEncoding)
gzw := newGzipResponseWriter(w)
defer gzw.Close()
w = gzw
Expand Down
7 changes: 4 additions & 3 deletions pkg/server/server_test.go
View file
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/metric"
Expand Down Expand Up @@ -199,7 +200,7 @@ func TestAcceptEncoding(t *testing.T) {
return b
},
},
{util.GzipEncoding,
{httputil.GzipEncoding,
func(b io.Reader) io.Reader {
r, err := gzip.NewReader(b)
if err != nil {
Expand All @@ -215,14 +216,14 @@ func TestAcceptEncoding(t *testing.T) {
t.Fatalf("could not create request: %s", err)
}
if d.acceptEncoding != "" {
req.Header.Set(util.AcceptEncodingHeader, d.acceptEncoding)
req.Header.Set(httputil.AcceptEncodingHeader, d.acceptEncoding)
}
resp, err := client.Do(req)
if err != nil {
t.Fatalf("could not make request to %s: %s", req.URL, err)
}
defer resp.Body.Close()
if ce := resp.Header.Get(util.ContentEncodingHeader); ce != d.acceptEncoding {
if ce := resp.Header.Get(httputil.ContentEncodingHeader); ce != d.acceptEncoding {
t.Fatalf("unexpected content encoding: '%s' != '%s'", ce, d.acceptEncoding)
}
r := d.newReader(resp.Body)
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/status.go
View file
Expand Up @@ -44,7 +44,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/server/status"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -547,7 +547,7 @@ func (s *statusServer) RaftDebug(
}

func (s *statusServer) handleVars(w http.ResponseWriter, r *http.Request) {
w.Header().Set(util.ContentTypeHeader, util.PlaintextContentType)
w.Header().Set(httputil.ContentTypeHeader, httputil.PlaintextContentType)
err := s.metricSource.PrintAsText(w)
if err != nil {
log.Error(r.Context(), err)
Expand Down
5 changes: 3 additions & 2 deletions pkg/server/status_test.go
View file
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/ts"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/stop"
Expand Down Expand Up @@ -78,7 +79,7 @@ func TestStatusLocalStacks(t *testing.T) {
}

// TestStatusJson verifies that status endpoints return expected Json results.
// The content type of the responses is always util.JSONContentType.
// The content type of the responses is always httputil.JSONContentType.
func TestStatusJson(t *testing.T) {
defer leaktest.AfterTest(t)()
s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
Expand Down Expand Up @@ -497,7 +498,7 @@ func TestSpanStatsResponse(t *testing.T) {
}

url := ts.AdminURL() + statusPrefix + "span"
if err := util.PostJSON(httpClient, url, &request, &response); err != nil {
if err := httputil.PostJSON(httpClient, url, &request, &response); err != nil {
t.Fatal(err)
}
if a, e := int(response.RangeCount), ExpectedInitialRangeCount(); a != e {
Expand Down
6 changes: 3 additions & 3 deletions pkg/testutils/serverutils/test_server_shim.go
View file
Expand Up @@ -42,8 +42,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/rpc"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/httputil"
"github.com/cockroachdb/cockroach/pkg/util/stop"
)

Expand Down Expand Up @@ -161,7 +161,7 @@ func GetJSONProto(ts TestServerInterface, path string, response proto.Message) e
if err != nil {
return err
}
return util.GetJSON(httpClient, ts.AdminURL()+path, response)
return httputil.GetJSON(httpClient, ts.AdminURL()+path, response)
}

// PostJSONProto uses the supplied client to POST request to the URL specified by
Expand All @@ -171,7 +171,7 @@ func PostJSONProto(ts TestServerInterface, path string, request, response proto.
if err != nil {
return err
}
return util.PostJSON(httpClient, ts.AdminURL()+path, request, response)
return httputil.PostJSON(httpClient, ts.AdminURL()+path, request, response)
}

// LookupRange returns the descriptor of the range containing key.
Expand Down

0 comments on commit 5769d35

Please sign in to comment.