Skip to content

Commit 4965ba7

Browse files
mdlayhermdlayher
authored
Merge pull request #21 from digitalocean/mdl-ovsdb-str-id
ovsdb: change IDs to strings in preparation for echo replies
2 parents 8957cde + ffa2d9c commit 4965ba7

File tree

5 files changed

+36
-32
lines changed

5 files changed

+36
-32
lines changed

‎ovsdb/client.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"io"
2121
"log"
2222
"net"
23+
"strconv"
2324
"strings"
2425
"sync"
2526
"sync/atomic"
@@ -40,7 +41,7 @@ type Client struct {
4041

4142
// Callbacks for RPC responses.
4243
cbMu sync.RWMutex
43-
callbacks map[int]callback
44+
callbacks map[string]callback
4445

4546
// Interval at which echo RPCs should occur in the background, and statistics
4647
// about the echo loop.
@@ -102,7 +103,7 @@ func New(conn net.Conn, options ...OptionFunc) (*Client, error) {
102103
client.c = jsonrpc.NewConn(conn, client.ll)
103104

104105
// Set up callbacks.
105-
client.callbacks = make(map[int]callback)
106+
client.callbacks = make(map[string]callback)
106107

107108
// Start up any background routines, and enable canceling them via context.
108109
ctx, cancel := context.WithCancel(context.Background())
@@ -136,8 +137,10 @@ func New(conn net.Conn, options ...OptionFunc) (*Client, error) {
136137
}
137138

138139
// requestID returns the next available request ID for an RPC.
139-
func (c *Client) requestID() int {
140-
return int(atomic.AddInt64(c.rpcID, 1))
140+
func (c *Client) requestID() string {
141+
// We use integer IDs by convention, but OVSDB happily accepts
142+
// any non-null JSON value.
143+
return strconv.FormatInt(atomic.AddInt64(c.rpcID, 1), 10)
141144
}
142145

143146
// Close closes a Client's connection and cleans up its resources.
@@ -322,21 +325,21 @@ type callback struct {
322325
}
323326

324327
// addCallback registers a callback for an RPC response for the specified ID.
325-
func (c *Client) addCallback(id int, cb callback) {
328+
func (c *Client) addCallback(id string, cb callback) {
326329
c.cbMu.Lock()
327330
defer c.cbMu.Unlock()
328331

329332
if _, ok := c.callbacks[id]; ok {
330333
// This ID was already registered.
331-
panicf("OVSDB callback with ID %d already registered", id)
334+
panicf("OVSDB callback with ID %q already registered", id)
332335
}
333336

334337
c.callbacks[id] = cb
335338
}
336339

337340
// doCallback performs a callback for an RPC response and clears the
338341
// callback on completion.
339-
func (c *Client) doCallback(id int, res rpcResponse) {
342+
func (c *Client) doCallback(id string, res rpcResponse) {
340343
c.cbMu.Lock()
341344
defer c.cbMu.Unlock()
342345

‎ovsdb/client_test.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"fmt"
2121
"log"
2222
"os"
23+
"strconv"
2324
"sync/atomic"
2425
"testing"
2526
"time"
@@ -34,7 +35,7 @@ func TestClientJSONRPCError(t *testing.T) {
3435

3536
c, _, done := testClient(t, func(_ jsonrpc.Request) jsonrpc.Response {
3637
return jsonrpc.Response{
37-
ID: intPtr(1),
38+
ID: strPtr("1"),
3839
Error: str,
3940
}
4041
})
@@ -51,7 +52,7 @@ func TestClientOVSDBError(t *testing.T) {
5152

5253
c, _, done := testClient(t, func(_ jsonrpc.Request) jsonrpc.Response {
5354
return jsonrpc.Response{
54-
ID: intPtr(1),
55+
ID: strPtr("1"),
5556
Result: mustMarshalJSON(t, &ovsdb.Error{
5657
Err: str,
5758
Details: "malformed",
@@ -79,7 +80,7 @@ func TestClientOVSDBError(t *testing.T) {
7980
func TestClientBadCallback(t *testing.T) {
8081
c, notifC, done := testClient(t, func(_ jsonrpc.Request) jsonrpc.Response {
8182
return jsonrpc.Response{
82-
ID: intPtr(1),
83+
ID: strPtr("1"),
8384
Result: mustMarshalJSON(t, []string{"foo"}),
8485
}
8586
})
@@ -88,7 +89,7 @@ func TestClientBadCallback(t *testing.T) {
8889
// Client doesn't have a callback for this ID.
8990
notifC <- &jsonrpc.Response{
9091
Method: "crash",
91-
ID: intPtr(10),
92+
ID: strPtr("foo"),
9293
}
9394

9495
if _, err := c.ListDatabases(context.Background()); err != nil {
@@ -103,7 +104,7 @@ func TestClientContextCancelBeforeRPC(t *testing.T) {
103104

104105
c, _, done := testClient(t, func(_ jsonrpc.Request) jsonrpc.Response {
105106
return jsonrpc.Response{
106-
ID: intPtr(1),
107+
ID: strPtr("1"),
107108
Result: mustMarshalJSON(t, []string{"foo"}),
108109
}
109110
})
@@ -133,7 +134,7 @@ func TestClientContextCancelDuringRPC(t *testing.T) {
133134
time.Sleep(500 * time.Millisecond)
134135

135136
return jsonrpc.Response{
136-
ID: intPtr(1),
137+
ID: strPtr("1"),
137138
Result: mustMarshalJSON(t, []string{"foo"}),
138139
}
139140
})
@@ -153,7 +154,7 @@ func TestClientLeakCallbacks(t *testing.T) {
153154
c, _, done := testClient(t, func(_ jsonrpc.Request) jsonrpc.Response {
154155
// Only respond with messages that don't match an incoming request.
155156
return jsonrpc.Response{
156-
ID: intPtr(100),
157+
ID: strPtr("foo"),
157158
Result: mustMarshalJSON(t, []string{"foo"}),
158159
}
159160
})
@@ -198,7 +199,7 @@ func TestClientEchoLoop(t *testing.T) {
198199
}
199200

200201
// Keep incrementing the request ID to match the client.
201-
id := int(atomic.AddInt64(&reqID, 1))
202+
id := strconv.Itoa(int(atomic.AddInt64(&reqID, 1)))
202203
return jsonrpc.Response{
203204
ID: &id,
204205
Result: mustMarshalJSON(t, req.Params),
@@ -273,8 +274,8 @@ func mustMarshalJSON(t *testing.T, v interface{}) []byte {
273274
return b
274275
}
275276

276-
func intPtr(i int) *int {
277-
return &i
277+
func strPtr(s string) *string {
278+
return &s
278279
}
279280

280281
func panicf(format string, a ...interface{}) {

‎ovsdb/internal/jsonrpc/conn.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ import (
2525

2626
// A Request is a JSON-RPC request.
2727
type Request struct {
28-
ID int `json:"id"`
28+
ID string `json:"id"`
2929
Method string `json:"method"`
3030
Params []interface{} `json:"params"`
3131
}
3232

3333
// A Response is either a JSON-RPC response, or a JSON-RPC request notification.
3434
type Response struct {
3535
// Non-null for response; null for request notification.
36-
ID *int `json:"id"`
36+
ID *string `json:"id"`
3737

3838
// Response fields.
3939
Result json.RawMessage `json:"result,omitempty"`
@@ -91,8 +91,8 @@ func (c *Conn) Close() error {
9191

9292
// Send sends a single JSON-RPC request.
9393
func (c *Conn) Send(req Request) error {
94-
if req.ID == 0 {
95-
return errors.New("JSON-RPC request ID must be non-zero")
94+
if req.ID == "" {
95+
return errors.New("JSON-RPC request ID must not be empty")
9696
}
9797

9898
// Non-nil array required for ovsdb-server to reply.

‎ovsdb/internal/jsonrpc/conn_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,15 @@ func TestConnSendReceiveError(t *testing.T) {
5151

5252
c, _, done := jsonrpc.TestConn(t, func(_ jsonrpc.Request) jsonrpc.Response {
5353
return jsonrpc.Response{
54-
ID: intPtr(10),
54+
ID: strPtr("10"),
5555
Error: rpcError{
5656
Details: "some error",
5757
},
5858
}
5959
})
6060
defer done()
6161

62-
if err := c.Send(jsonrpc.Request{ID: 10}); err != nil {
62+
if err := c.Send(jsonrpc.Request{ID: "10"}); err != nil {
6363
t.Fatalf("failed to send request: %v", err)
6464
}
6565

@@ -77,7 +77,7 @@ func TestConnSendReceiveOK(t *testing.T) {
7777
req := jsonrpc.Request{
7878
Method: "hello",
7979
Params: []interface{}{"world"},
80-
ID: 1,
80+
ID: "1",
8181
}
8282

8383
type message struct {
@@ -94,7 +94,7 @@ func TestConnSendReceiveOK(t *testing.T) {
9494
}
9595

9696
return jsonrpc.Response{
97-
ID: intPtr(1),
97+
ID: strPtr("1"),
9898
Result: mustMarshalJSON(t, want),
9999
}
100100
})
@@ -124,7 +124,7 @@ func TestConnSendReceiveOK(t *testing.T) {
124124
}
125125

126126
func TestConnSendReceiveNotificationsOK(t *testing.T) {
127-
const id = 10
127+
const id = "10"
128128

129129
req := jsonrpc.Request{
130130
ID: id,
@@ -133,7 +133,7 @@ func TestConnSendReceiveNotificationsOK(t *testing.T) {
133133
}
134134

135135
res := jsonrpc.Response{
136-
ID: intPtr(id),
136+
ID: strPtr(id),
137137
Result: mustMarshalJSON(t, "some bytes"),
138138
}
139139

@@ -198,8 +198,8 @@ func mustMarshalJSON(t *testing.T, v interface{}) []byte {
198198
return b
199199
}
200200

201-
func intPtr(i int) *int {
202-
return &i
201+
func strPtr(s string) *string {
202+
return &s
203203
}
204204

205205
func panicf(format string, a ...interface{}) {

‎ovsdb/rpc_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestClientListDatabases(t *testing.T) {
3535
}
3636

3737
return jsonrpc.Response{
38-
ID: intPtr(1),
38+
ID: strPtr("1"),
3939
Result: mustMarshalJSON(t, want),
4040
}
4141
})
@@ -54,7 +54,7 @@ func TestClientListDatabases(t *testing.T) {
5454
func TestClientEchoError(t *testing.T) {
5555
c, _, done := testClient(t, func(req jsonrpc.Request) jsonrpc.Response {
5656
return jsonrpc.Response{
57-
ID: intPtr(1),
57+
ID: strPtr("1"),
5858
Result: mustMarshalJSON(t, []string{"foo"}),
5959
}
6060
})
@@ -78,7 +78,7 @@ func TestClientEchoOK(t *testing.T) {
7878
}
7979

8080
return jsonrpc.Response{
81-
ID: intPtr(1),
81+
ID: strPtr("1"),
8282
Result: mustMarshalJSON(t, []string{echo}),
8383
}
8484
})

0 commit comments

Comments
 (0)