rpc: implement full bi-directional communication (#18471)

New APIs added:

    client.RegisterName(namespace, service) // makes service available to server
    client.Notify(ctx, method, args...)     // sends a notification
    ClientFromContext(ctx)                  // to get a client in handler method

This is essentially a rewrite of the server-side code. JSON-RPC
processing code is now the same on both server and client side. Many
minor issues were fixed in the process and there is a new test suite for
JSON-RPC spec compliance (and non-compliance in some cases).

List of behavior changes:

- Method handlers are now called with a per-request context instead of a
  per-connection context. The context is canceled right after the method
  returns.
- Subscription error channels are always closed when the connection
  ends. There is no need to also wait on the Notifier's Closed channel
  to detect whether the subscription has ended.
- Client now omits "params" instead of sending "params": null when there
  are no arguments to a call. The previous behavior was not compliant
  with the spec. The server still accepts "params": null.
- Floating point numbers are allowed as "id". The spec doesn't allow
  them, but we handle request "id" as json.RawMessage and guarantee that
  the same number will be sent back.
- Logging is improved significantly. There is now a message at DEBUG
  level for each RPC call served.

1 files changed, 99 insertions, 109 deletions

diff --git a/rpc/server_test.go b/rpc/server_test.go
index 90d62f26d..39099546b 100644
--- a/rpc/server_test.go
+++ b/rpc/server_test.go
@@ -17,146 +17,136 @@
 package rpc
 
 import (
-	"context"
-	"encoding/json"
+	"bufio"
+	"bytes"
+	"io"
+	"io/ioutil"
 	"net"
-	"reflect"
+	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
 
-type Service struct{}
-
-type Args struct {
-	S string
-}
-
-func (s *Service) NoArgsRets() {
-}
-
-type Result struct {
-	String string
-	Int    int
-	Args   *Args
-}
-
-func (s *Service) Echo(str string, i int, args *Args) Result {
-	return Result{str, i, args}
-}
-
-func (s *Service) EchoWithCtx(ctx context.Context, str string, i int, args *Args) Result {
-	return Result{str, i, args}
-}
-
-func (s *Service) Sleep(ctx context.Context, duration time.Duration) {
-	select {
-	case <-time.After(duration):
-	case <-ctx.Done():
-	}
-}
-
-func (s *Service) Rets() (string, error) {
-	return "", nil
-}
-
-func (s *Service) InvalidRets1() (error, string) {
-	return nil, ""
-}
-
-func (s *Service) InvalidRets2() (string, string) {
-	return "", ""
-}
-
-func (s *Service) InvalidRets3() (string, string, error) {
-	return "", "", nil
-}
-
-func (s *Service) Subscription(ctx context.Context) (*Subscription, error) {
-	return nil, nil
-}
-
 func TestServerRegisterName(t *testing.T) {
 	server := NewServer()
-	service := new(Service)
+	service := new(testService)
 
-	if err := server.RegisterName("calc", service); err != nil {
+	if err := server.RegisterName("test", service); err != nil {
 		t.Fatalf("%v", err)
 	}
 
-	if len(server.services) != 2 {
-		t.Fatalf("Expected 2 service entries, got %d", len(server.services))
+	if len(server.services.services) != 2 {
+		t.Fatalf("Expected 2 service entries, got %d", len(server.services.services))
 	}
 
-	svc, ok := server.services["calc"]
+	svc, ok := server.services.services["test"]
 	if !ok {
 		t.Fatalf("Expected service calc to be registered")
 	}
 
-	if len(svc.callbacks) != 5 {
-		t.Errorf("Expected 5 callbacks for service 'calc', got %d", len(svc.callbacks))
-	}
-
-	if len(svc.subscriptions) != 1 {
-		t.Errorf("Expected 1 subscription for service 'calc', got %d", len(svc.subscriptions))
+	wantCallbacks := 7
+	if len(svc.callbacks) != wantCallbacks {
+		t.Errorf("Expected %d callbacks for service 'service', got %d", wantCallbacks, len(svc.callbacks))
 	}
 }
 
-func testServerMethodExecution(t *testing.T, method string) {
-	server := NewServer()
-	service := new(Service)
-
-	if err := server.RegisterName("test", service); err != nil {
-		t.Fatalf("%v", err)
+func TestServer(t *testing.T) {
+	files, err := ioutil.ReadDir("testdata")
+	if err != nil {
+		t.Fatal("where'd my testdata go?")
 	}
+	for _, f := range files {
+		if f.IsDir() || strings.HasPrefix(f.Name(), ".") {
+			continue
+		}
+		path := filepath.Join("testdata", f.Name())
+		name := strings.TrimSuffix(f.Name(), filepath.Ext(f.Name()))
+		t.Run(name, func(t *testing.T) {
+			runTestScript(t, path)
+		})
+	}
+}
 
-	stringArg := "string arg"
-	intArg := 1122
-	argsArg := &Args{"abcde"}
-	params := []interface{}{stringArg, intArg, argsArg}
-
-	request := map[string]interface{}{
-		"id":      12345,
-		"method":  "test_" + method,
-		"version": "2.0",
-		"params":  params,
+func runTestScript(t *testing.T, file string) {
+	server := newTestServer()
+	content, err := ioutil.ReadFile(file)
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	clientConn, serverConn := net.Pipe()
 	defer clientConn.Close()
-
-	go server.ServeCodec(NewJSONCodec(serverConn), OptionMethodInvocation)
-
-	out := json.NewEncoder(clientConn)
-	in := json.NewDecoder(clientConn)
-
-	if err := out.Encode(request); err != nil {
-		t.Fatal(err)
+	go server.ServeCodec(NewJSONCodec(serverConn), OptionMethodInvocation|OptionSubscriptions)
+	readbuf := bufio.NewReader(clientConn)
+	for _, line := range strings.Split(string(content), "\n") {
+		line = strings.TrimSpace(line)
+		switch {
+		case len(line) == 0 || strings.HasPrefix(line, "//"):
+			// skip comments, blank lines
+			continue
+		case strings.HasPrefix(line, "--> "):
+			t.Log(line)
+			// write to connection
+			clientConn.SetWriteDeadline(time.Now().Add(5 * time.Second))
+			if _, err := io.WriteString(clientConn, line[4:]+"\n"); err != nil {
+				t.Fatalf("write error: %v", err)
+			}
+		case strings.HasPrefix(line, "<-- "):
+			t.Log(line)
+			want := line[4:]
+			// read line from connection and compare text
+			clientConn.SetReadDeadline(time.Now().Add(5 * time.Second))
+			sent, err := readbuf.ReadString('\n')
+			if err != nil {
+				t.Fatalf("read error: %v", err)
+			}
+			sent = strings.TrimRight(sent, "\r\n")
+			if sent != want {
+				t.Errorf("wrong line from server\ngot:  %s\nwant: %s", sent, want)
+			}
+		default:
+			panic("invalid line in test script: " + line)
+		}
 	}
+}
 
-	response := jsonSuccessResponse{Result: &Result{}}
-	if err := in.Decode(&response); err != nil {
-		t.Fatal(err)
-	}
+// This test checks that responses are delivered for very short-lived connections that
+// only carry a single request.
+func TestServerShortLivedConn(t *testing.T) {
+	server := newTestServer()
+	defer server.Stop()
 
-	if result, ok := response.Result.(*Result); ok {
-		if result.String != stringArg {
-			t.Errorf("expected %s, got : %s\n", stringArg, result.String)
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal("can't listen:", err)
+	}
+	defer listener.Close()
+	go server.ServeListener(listener)
+
+	var (
+		request  = `{"jsonrpc":"2.0","id":1,"method":"rpc_modules"}` + "\n"
+		wantResp = `{"jsonrpc":"2.0","id":1,"result":{"nftest":"1.0","rpc":"1.0","test":"1.0"}}` + "\n"
+		deadline = time.Now().Add(10 * time.Second)
+	)
+	for i := 0; i < 20; i++ {
+		conn, err := net.Dial("tcp", listener.Addr().String())
+		if err != nil {
+			t.Fatal("can't dial:", err)
 		}
-		if result.Int != intArg {
-			t.Errorf("expected %d, got %d\n", intArg, result.Int)
+		defer conn.Close()
+		conn.SetDeadline(deadline)
+		// Write the request, then half-close the connection so the server stops reading.
+		conn.Write([]byte(request))
+		conn.(*net.TCPConn).CloseWrite()
+		// Now try to get the response.
+		buf := make([]byte, 2000)
+		n, err := conn.Read(buf)
+		if err != nil {
+			t.Fatal("read error:", err)
 		}
-		if !reflect.DeepEqual(result.Args, argsArg) {
-			t.Errorf("expected %v, got %v\n", argsArg, result)
+		if !bytes.Equal(buf[:n], []byte(wantResp)) {
+			t.Fatalf("wrong response: %s", buf[:n])
 		}
-	} else {
-		t.Fatalf("invalid response: expected *Result - got: %T", response.Result)
 	}
 }
-
-func TestServerMethodExecution(t *testing.T) {
-	testServerMethodExecution(t, "echo")
-}
-
-func TestServerMethodWithCtx(t *testing.T) {
-	testServerMethodExecution(t, "echoWithCtx")
-}