chore(go): make Load call idempotent (#2645)

Making the `Load` runtime library call idempotent makes it so that the
`Client` can be fully disposed (via the `Close` call), while
subsequently getting a new client starte up if needed.

This is in particular useful for implementing test suites that operate
on a clean kernel instance each time.

This entails making sure the registered type information (as in the
`typeregistry.TypeRegistry` instance) survives `*Client.Close()`, as
the `init` functions will not re-trigger.


---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

packages/@jsii/go-runtime/jsii-runtime-go/internal/kernel/callbacks.go

@@ -143,4 +143,4 @@ func (c *Client) invoke(method reflect.Value, args []interface{}) (retval reflec
 		err = fmt.Errorf("too many return values: %v", result)
 	}
 	return
-}
+}

packages/@jsii/go-runtime/jsii-runtime-go/internal/kernel/client.go

@@ -16,6 +16,7 @@ var (
 	clientInstance      *Client
 	clientInstanceMutex sync.Mutex
 	clientOnce          sync.Once
+	types               *typeregistry.TypeRegistry = typeregistry.New()
 )
 
 // The Client struct owns the jsii child process and its io interfaces. It also
@@ -24,8 +25,10 @@ var (
 // process by reference.
 type Client struct {
 	process *process.Process
-	types   *typeregistry.TypeRegistry
 	objects *objectstore.ObjectStore
+
+	// Supports the idempotency of the Load method.
+	loaded map[LoadProps]LoadResponse
 }
 
 // GetClient returns a singleton Client instance, initializing one the first
@@ -68,16 +71,15 @@ func CloseClient() {
 	}
 }
 
-// newClient starts the kernel child process and verifies the "hello" message
-// was correct.
+// newClient initializes a client, making it ready for business.
 func newClient() (*Client, error) {
 	if process, err := process.NewProcess(fmt.Sprintf("^%s", version)); err != nil {
 		return nil, err
 	} else {
 		result := &Client{
 			process: process,
 			objects: objectstore.New(),
-			types:   typeregistry.New(),
+			loaded:  make(map[LoadProps]LoadResponse),
 		}
 
 		// Register a finalizer to call Close()
@@ -90,7 +92,7 @@ func newClient() (*Client, error) {
 }
 
 func (c *Client) Types() *typeregistry.TypeRegistry {
-	return c.types
+	return types
 }
 
 func (c *Client) RegisterInstance(instance reflect.Value, instanceID string) error {
@@ -131,4 +133,4 @@ func (c *Client) close() {
 
 	// We no longer need a finalizer to run
 	runtime.SetFinalizer(c, nil)
-}
+}

packages/@jsii/go-runtime/jsii-runtime-go/internal/kernel/client_test.go

@@ -1,30 +1,61 @@
 package kernel
 
 import (
-	"fmt"
+	"github.com/aws/jsii-runtime-go/internal/typeregistry"
+	"reflect"
 	"testing"
 )
 
 func TestClient(t *testing.T) {
 	client, err := newClient()
 	if err != nil {
-		t.Log(err)
-		panic(fmt.Sprintf("Client init error: %s", err.Error()))
+		t.Fatalf("client init failed: %s", err.Error())
 	}
 	defer client.close()
 
 	t.Run("Client Load Error", func(t *testing.T) {
 		request := LoadProps{
 			Name:    "jsii-calc",
 			Version: "0.0.0",
-			Tarball: "jsii-calc-tarball.tgz",
 		}
 
-		res, err := client.Load(request)
+		res, err := client.Load(request, nil)
 
 		t.Log(res)
 		if err != nil {
 			t.Log(err)
 		}
 	})
+
+	t.Run("Type registry survives CloseClient()", func(t *testing.T) {
+		client, err := newClient()
+		if err != nil {
+			t.Fatalf("client init failed: %s", err.Error())
+		}
+
+		// Clean up after ourselves, so this test does not leave traces behind.
+		defer func() { types = typeregistry.New() }()
+
+		type enumType string
+		var enumTypeFOO enumType = "FOO"
+
+		registry := client.Types()
+		err = registry.RegisterEnum(
+			"example.Enum",
+			reflect.TypeOf((*enumType)(nil)).Elem(),
+			map[string]interface{}{"FOO": enumTypeFOO},
+		)
+		if err != nil {
+			t.Fatalf("failed registering enum: %s", err.Error())
+		}
+
+		CloseClient()
+
+		// Getting the registry from the client again.
+		registry = client.Types()
+
+		if _, ok := registry.TryRenderEnumRef(reflect.ValueOf(enumTypeFOO)); !ok {
+			t.Errorf("failed rendering enum ref, it should have worked!")
+		}
+	})
 }

packages/@jsii/go-runtime/jsii-runtime-go/internal/kernel/json.go

@@ -37,4 +37,4 @@ func unmarshalKernelResponse(data []byte, uresult kernelResponder, result kernel
 	}
 
 	return json.Unmarshal(response["ok"], uresult)
-}
+}

packages/@jsii/go-runtime/jsii-runtime-go/internal/kernel/load.go

@@ -1,11 +1,17 @@
 package kernel
 
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"regexp"
+)
+
 // LoadProps holds the necessary information to load a library into the
 // @jsii/kernel process through the Load method.
 type LoadProps struct {
 	Name    string `json:"name"`
 	Version string `json:"version"`
-	Tarball string `json:"tarball"`
 }
 
 // LoadResponse contains the data returned by the @jsii/kernel process in
@@ -16,12 +22,39 @@ type LoadResponse struct {
 	Types    float64 `json:"types"`
 }
 
-func (c *Client) Load(props LoadProps) (response LoadResponse, err error) {
+// Load ensures the specified assembly has been loaded into the @jsii/kernel
+// process. This call is idempotent (calling it several times with the same
+// input results in the same output).
+func (c *Client) Load(props LoadProps, tarball []byte) (response LoadResponse, err error) {
+	if response, cached := c.loaded[props]; cached {
+		return response, nil
+	}
+
+	tmpfile, err := ioutil.TempFile("", fmt.Sprintf(
+		"%s-%s.*.tgz",
+		regexp.MustCompile("[^a-zA-Z0-9_-]").ReplaceAllString(props.Name, "-"),
+		version,
+	))
+	if err != nil {
+		return
+	}
+	defer os.Remove(tmpfile.Name())
+	if _, err := tmpfile.Write(tarball); err != nil {
+		panic(err)
+	}
+	tmpfile.Close()
+
 	type request struct {
 		kernelRequest
 		LoadProps
+		Tarball string `json:"tarball"`
+	}
+	err = c.request(request{kernelRequest{"load"}, props, tmpfile.Name()}, &response)
+
+	if err == nil {
+		c.loaded[props] = response
 	}
-	err = c.request(request{kernelRequest{"load"}, props}, &response)
+
 	return
 }
 

packages/@jsii/go-runtime/jsii-runtime-go/internal/typeregistry/registration.go

@@ -22,6 +22,11 @@ type registeredType struct {
 	Kind typeKind
 }
 
+type registeredStruct struct {
+	FQN    api.FQN
+	Fields []reflect.StructField
+}
+
 // RegisterClass maps the given FQN to the provided class interface, list of
 // overrides, and proxy maker function. This returns an error if the class
 // type is not a go interface.
@@ -56,6 +61,9 @@ func (t *TypeRegistry) RegisterEnum(fqn api.FQN, enm reflect.Type, members map[s
 	if existing, exists := t.fqnToType[fqn]; exists && existing.Type != enm {
 		return fmt.Errorf("another type was already registered with %s: %v", fqn, existing)
 	}
+	if existing, exists := t.typeToEnumFQN[enm]; exists && existing != fqn {
+		return fmt.Errorf("attempted to re-register %v as %s, but it was registered as %s", enm, fqn, existing)
+	}
 	for memberName, memberVal := range members {
 		vt := reflect.ValueOf(memberVal).Type()
 		if vt != enm {
@@ -110,10 +118,6 @@ func (t *TypeRegistry) RegisterStruct(fqn api.FQN, strct reflect.Type) error {
 		return fmt.Errorf("another type was already registered with %s: %v", fqn, existing)
 	}
 
-	if existing, exists := t.typeToFQN[strct]; exists && existing != fqn {
-		return fmt.Errorf("attempting to register type %s as %s, but it was already registered as: %s", strct.String(), fqn, existing)
-	}
-
 	fields := []reflect.StructField{}
 	numField := strct.NumField()
 	for i := 0; i < numField; i++ {
@@ -131,8 +135,7 @@ func (t *TypeRegistry) RegisterStruct(fqn api.FQN, strct reflect.Type) error {
 	}
 
 	t.fqnToType[fqn] = registeredType{strct, structType}
-	t.typeToFQN[strct] = fqn
-	t.structFields[strct] = fields
+	t.structInfo[strct] = registeredStruct{FQN: fqn, Fields: fields}
 
 	return nil
 }

packages/@jsii/go-runtime/jsii-runtime-go/internal/typeregistry/type-registry.go → packages/@jsii/go-runtime/jsii-runtime-go/internal/typeregistry/typeregistry.go

@@ -18,37 +18,32 @@ type TypeRegistry struct {
 	// enums are not included
 	fqnToType map[api.FQN]registeredType
 
-	// typeToFQN is sued to obtain the jsii fully qualified type name for a
-	// given native go type. Currently only tracks jsii struct types.
-	typeToFQN map[reflect.Type]api.FQN
-
-	// map enum member FQNs (e.g. "jsii-calc.StringEnum/A") to the corresponding
-	// go const for this member.
+	// fqnToEnumMember maps enum member FQNs (e.g. "jsii-calc.StringEnum/A") to
+	// the corresponding go const for this member.
 	fqnToEnumMember map[string]interface{}
 
-	// maps Go enum type ("StringEnum") to the corresponding jsii enum FQN (e.g.
-	// "jsii-calc.StringEnum")
+	// typeToEnumFQN maps Go enum type ("StringEnum") to the corresponding jsii
+	// enum FQN (e.g. "jsii-calc.StringEnum")
 	typeToEnumFQN map[reflect.Type]api.FQN
 
-	// maps registered struct types to all their fields.
-	structFields map[reflect.Type][]reflect.StructField
+	// structInfo maps registered struct types to all their fields.
+	structInfo map[reflect.Type]registeredStruct
 
-	// map registered interface types to a proxy maker function
+	// proxyMakers map registered interface types to a proxy maker function.
 	proxyMakers map[reflect.Type]func() interface{}
 
-	// typeMembers maps each class or interface FQN to the set of members it implements
-	// in the form of api.Override values.
+	// typeMembers maps each class or interface FQN to the set of members it
+	// implements in the form of api.Override values.
 	typeMembers map[api.FQN][]api.Override
 }
 
 // New creates a new type registry.
 func New() *TypeRegistry {
 	return &TypeRegistry{
 		fqnToType:       make(map[api.FQN]registeredType),
-		typeToFQN:       make(map[reflect.Type]api.FQN),
 		fqnToEnumMember: make(map[string]interface{}),
 		typeToEnumFQN:   make(map[reflect.Type]api.FQN),
-		structFields:    make(map[reflect.Type][]reflect.StructField),
+		structInfo:      make(map[reflect.Type]registeredStruct),
 		proxyMakers:     make(map[reflect.Type]func() interface{}),
 		typeMembers:     make(map[api.FQN][]api.Override),
 	}
@@ -58,15 +53,12 @@ func New() *TypeRegistry {
 // the jsii fully qualified type name, and a boolean telling whether the
 // provided type was a registered jsii struct type.
 func (t *TypeRegistry) StructFields(typ reflect.Type) (fields []reflect.StructField, fqn api.FQN, ok bool) {
-	if fqn, ok = t.typeToFQN[typ]; !ok {
+	var info registeredStruct
+	if info, ok = t.structInfo[typ]; !ok {
 		return
 	}
-
-	var found []reflect.StructField
-	if found, ok = t.structFields[typ]; ok {
-		// Returning a copy, to ensure our storage does not get mutated.
-		fields = append(fields, found...)
-	}
+	fqn = info.FQN
+	fields = append(fields, info.Fields...)
 	return
 }
 

packages/@jsii/go-runtime/jsii-runtime-go/runtime.go

@@ -2,10 +2,7 @@ package jsii
 
 import (
 	"fmt"
-	"io/ioutil"
-	"os"
 	"reflect"
-	"regexp"
 
 	"github.com/aws/jsii-runtime-go/internal/api"
 	"github.com/aws/jsii-runtime-go/internal/kernel"
@@ -37,25 +34,10 @@ func (m MemberProperty) toOverride() api.Override {
 func Load(name string, version string, tarball []byte) {
 	c := kernel.GetClient()
 
-	tmpfile, err := ioutil.TempFile("", fmt.Sprintf(
-		"%s-%s.*.tgz",
-		regexp.MustCompile("[^a-zA-Z0-9_-]").ReplaceAllString(name, "-"),
-		version,
-	))
-	if err != nil {
-		panic(err)
-	}
-	defer os.Remove(tmpfile.Name())
-	if _, err := tmpfile.Write(tarball); err != nil {
-		panic(err)
-	}
-	tmpfile.Close()
-
-	_, err = c.Load(kernel.LoadProps{
+	_, err := c.Load(kernel.LoadProps{
 		Name:    name,
 		Version: version,
-		Tarball: tmpfile.Name(),
-	})
+	}, tarball)
 	if err != nil {
 		panic(err)
 	}

packages/@jsii/go-runtime/package.json

@@ -11,7 +11,7 @@
     "go:run": "go mod download && node build-tools/go-run.js",
     "package": "build-tools/package.sh",
     "doc": "(cd jsii-runtime-go && yarn --silent go:run godoc)",
-    "fmt": "(cd jsii-runtime-go && yarn --silent go:all yarn --silent go:run goimports .)",
+    "fmt": "yarn --silent go:run goimports -w jsii-runtime-go",
     "lint": "(cd jsii-runtime-go && go vet ./... && yarn --silent go:run golint ./...)",
     "test": "(cd jsii-runtime-go && go test ./...)"
   },