Clean up method receiver constructors

This removes duplicate constructors for Context and Receiver method
receivers, enforcing instantiation through the canonical Engine methods.

engine/context.go

@@ -9,6 +9,7 @@ package engine
 //
 // #include <stdlib.h>
 // #include <main/php.h>
+// #include "receiver.h"
 // #include "context.h"
 import "C"
 
@@ -30,8 +31,9 @@ type Context struct {
 	// Header represents the HTTP headers set by current PHP context.
 	Header http.Header
 
-	context *C.struct__engine_context
-	values  []*Value
+	context   *C.struct__engine_context
+	values    []*Value
+	receivers map[string]*Receiver
 }
 
 // Bind allows for binding Go values into the current execution context under
@@ -53,6 +55,34 @@ func (c *Context) Bind(name string, val interface{}) error {
 	return nil
 }
 
+// Define registers a PHP class for the name passed, using function fn as
+// constructor for individual object instances as needed by the PHP context.
+//
+// The class name registered is assumed to be unique for the active engine.
+//
+// The constructor function accepts a slice of arguments, as passed by the PHP
+// context, and should return a method receiver instance, or nil on error (in
+// which case, an exception is thrown on the PHP object constructor).
+func (c *Context) Define(name string, fn func(args []interface{}) interface{}) error {
+	if _, exists := c.receivers[name]; exists {
+		return fmt.Errorf("Failed to define duplicate receiver '%s'", name)
+	}
+
+	rcvr := &Receiver{
+		name:    name,
+		create:  fn,
+		objects: make([]*ReceiverObject, 0),
+	}
+
+	n := C.CString(name)
+	defer C.free(unsafe.Pointer(n))
+
+	C.receiver_define(n, unsafe.Pointer(rcvr))
+	c.receivers[name] = rcvr
+
+	return nil
+}
+
 // Exec executes a PHP script pointed to by filename in the current execution
 // context, and returns an error, if any. Output produced by the script is
 // written to the context's pre-defined io.Writer instance.
@@ -99,6 +129,12 @@ func (c *Context) Destroy() {
 		return
 	}
 
+	for _, r := range c.receivers {
+		r.Destroy()
+	}
+
+	c.receivers = nil
+
 	for _, v := range c.values {
 		v.Destroy()
 	}

engine/context_test.go

@@ -31,6 +31,28 @@ func TestContextNew(t *testing.T) {
 	c.Destroy()
 }
 
+func TestContextDefine(t *testing.T) {
+	ctor := func(args []interface{}) interface{} {
+		return nil
+	}
+
+	c, _ := e.NewContext()
+
+	if err := c.Define("TestDefine", ctor); err != nil {
+		t.Errorf("Context.Define(): %s", err)
+	}
+
+	if len(c.receivers) != 1 {
+		t.Errorf("Context.Define(): `Context.receivers` length is %d, should be 1", len(c.receivers))
+	}
+
+	if err := c.Define("TestDefine", ctor); err == nil {
+		t.Errorf("Context.Define(): Incorrectly defined duplicate receiver")
+	}
+
+	c.Destroy()
+}
+
 var execTests = []struct {
 	name     string
 	script   string

engine/engine.go

@@ -25,9 +25,8 @@ import (
 
 // Engine represents the core PHP engine bindings.
 type Engine struct {
-	engine    *C.struct__php_engine
-	contexts  []*Context
-	receivers map[string]*Receiver
+	engine   *C.struct__php_engine
+	contexts []*Context
 }
 
 // New initializes a PHP engine instance on which contexts can be executed. It
@@ -39,9 +38,8 @@ func New() (*Engine, error) {
 	}
 
 	e := &Engine{
-		engine:    ptr,
-		contexts:  make([]*Context, 0),
-		receivers: make(map[string]*Receiver),
+		engine:   ptr,
+		contexts: make([]*Context, 0),
 	}
 
 	return e, nil
@@ -52,8 +50,9 @@ func New() (*Engine, error) {
 // corresponds to PHP's RINIT (request init) phase.
 func (e *Engine) NewContext() (*Context, error) {
 	ctx := &Context{
-		Header: make(http.Header),
-		values: make([]*Value, 0),
+		Header:    make(http.Header),
+		values:    make([]*Value, 0),
+		receivers: make(map[string]*Receiver),
 	}
 
 	ptr, err := C.context_new(unsafe.Pointer(ctx))
@@ -67,35 +66,12 @@ func (e *Engine) NewContext() (*Context, error) {
 	return ctx, nil
 }
 
-// Define registers a PHP class under the name passed, using fn as the class
-// constructor.
-func (e *Engine) Define(name string, fn func(args []interface{}) interface{}) error {
-	if _, exists := e.receivers[name]; exists {
-		return fmt.Errorf("Failed to define duplicate receiver '%s'", name)
-	}
-
-	rcvr, err := NewReceiver(name, fn)
-	if err != nil {
-		return err
-	}
-
-	e.receivers[name] = rcvr
-
-	return nil
-}
-
 // Destroy shuts down and frees any resources related to the PHP engine bindings.
 func (e *Engine) Destroy() {
 	if e.engine == nil {
 		return
 	}
 
-	for _, r := range e.receivers {
-		r.Destroy()
-	}
-
-	e.receivers = nil
-
 	for _, c := range e.contexts {
 		c.Destroy()
 	}

engine/engine_test.go

@@ -44,7 +44,7 @@ func TestEngineNew(t *testing.T) {
 		t.Fatalf("New(): %s", err)
 	}
 
-	if e.engine == nil || e.contexts == nil || e.receivers == nil {
+	if e.engine == nil || e.contexts == nil {
 		t.Fatalf("New(): Struct fields are `nil` but no error returned")
 	}
 }
@@ -60,31 +60,10 @@ func TestEngineNewContext(t *testing.T) {
 	}
 }
 
-func TestEngineDefine(t *testing.T) {
-	ctor := func(args []interface{}) interface{} {
-		return nil
-	}
-
-	err := e.Define("TestDefine", ctor)
-	if err != nil {
-		t.Errorf("Define(): %s", err)
-	}
-
-	if len(e.receivers) != 1 {
-		t.Errorf("Define(): `Engine.receivers` length is %d, should be 1", len(e.receivers))
-	}
-
-	err = e.Define("TestDefine", ctor)
-	if err == nil {
-		t.Errorf("Define(): Incorrectly defined duplicate receiver")
-	}
-
-}
-
 func TestEngineDestroy(t *testing.T) {
 	e.Destroy()
 
-	if e.engine != nil || e.contexts != nil || e.receivers != nil {
+	if e.engine != nil || e.contexts != nil {
 		t.Errorf("Engine.Destroy(): Did not set internal fields to `nil`")
 	}
 

engine/receiver.go

@@ -17,7 +17,8 @@ import (
 	"unsafe"
 )
 
-type object struct {
+// ReceiverObject represents an object instance of a pre-defined method receiver.
+type ReceiverObject struct {
 	instance interface{}
 	values   map[string]reflect.Value
 	methods  map[string]reflect.Value
@@ -27,30 +28,7 @@ type object struct {
 type Receiver struct {
 	name    string
 	create  func(args []interface{}) interface{}
-	objects []*object
-}
-
-// NewReceiver registers a PHP class for the name passed, using function fn as
-// constructor for individual object instances as needed by the PHP context.
-//
-// The class name registered is assumed to be unique for the active engine.
-//
-// The constructor function accepts a slice of arguments, as passed by the PHP
-// context, and should return a method receiver instance, or nil on error (in
-// which case, an exception is thrown on the PHP object constructor).
-func NewReceiver(name string, fn func(args []interface{}) interface{}) (*Receiver, error) {
-	rcvr := &Receiver{
-		name:    name,
-		create:  fn,
-		objects: make([]*object, 0),
-	}
-
-	n := C.CString(name)
-	defer C.free(unsafe.Pointer(n))
-
-	C.receiver_define(n, unsafe.Pointer(rcvr))
-
-	return rcvr, nil
+	objects []*ReceiverObject
 }
 
 // Destroy removes references to the generated PHP class for this receiver and
@@ -80,7 +58,7 @@ func receiverNew(rcvr unsafe.Pointer, args unsafe.Pointer) unsafe.Pointer {
 
 	defer va.Destroy()
 
-	obj := &object{
+	obj := &ReceiverObject{
 		instance: r.create(va.Slice()),
 		values:   make(map[string]reflect.Value),
 		methods:  make(map[string]reflect.Value),
@@ -120,7 +98,7 @@ func receiverNew(rcvr unsafe.Pointer, args unsafe.Pointer) unsafe.Pointer {
 
 //export receiverGet
 func receiverGet(obj unsafe.Pointer, name *C.char) unsafe.Pointer {
-	o := (*object)(obj)
+	o := (*ReceiverObject)(obj)
 	n := C.GoString(name)
 
 	if _, exists := o.values[n]; !exists || !o.values[n].CanInterface() {
@@ -137,7 +115,7 @@ func receiverGet(obj unsafe.Pointer, name *C.char) unsafe.Pointer {
 
 //export receiverSet
 func receiverSet(obj unsafe.Pointer, name *C.char, val unsafe.Pointer) {
-	o := (*object)(obj)
+	o := (*ReceiverObject)(obj)
 	n := C.GoString(name)
 
 	// Do not attempt to set non-existing or unset-able field.
@@ -155,7 +133,7 @@ func receiverSet(obj unsafe.Pointer, name *C.char, val unsafe.Pointer) {
 
 //export receiverExists
 func receiverExists(obj unsafe.Pointer, name *C.char) C.int {
-	o := (*object)(obj)
+	o := (*ReceiverObject)(obj)
 	n := C.GoString(name)
 
 	if _, exists := o.values[n]; !exists {
@@ -167,7 +145,7 @@ func receiverExists(obj unsafe.Pointer, name *C.char) C.int {
 
 //export receiverCall
 func receiverCall(obj unsafe.Pointer, name *C.char, args unsafe.Pointer) unsafe.Pointer {
-	o := (*object)(obj)
+	o := (*ReceiverObject)(obj)
 	n := C.GoString(name)
 
 	if _, exists := o.methods[n]; !exists {

engine/receiver_test.go

@@ -49,7 +49,7 @@ func newTestReceiver(args []interface{}) interface{} {
 	return &testReceiver{Var: value, hidden: 42}
 }
 
-var newReceiverTests = []struct {
+var receiverDefineTests = []struct {
 	script   string
 	expected string
 }{
@@ -65,7 +65,6 @@ var newReceiverTests = []struct {
 		}`,
 		"Failed to instantiate method receiver",
 	},
-
 	{
 		"$t = new TestReceiver; echo $t->Var;",
 		"Foo",
@@ -78,7 +77,6 @@ var newReceiverTests = []struct {
 		"$t = new TestReceiver('wow'); echo $t->Var;",
 		"wow",
 	},
-
 	{
 		"$t = new TestReceiver; $t->Var = 'Bar'; echo $t->Var;",
 		"Bar",
@@ -87,7 +85,6 @@ var newReceiverTests = []struct {
 		"$t = new TestReceiver; $t->hello = 'wow'; echo $t->hello;",
 		"",
 	},
-
 	{
 		"$t = new TestReceiver; echo $t->Ignore();",
 		"",
@@ -104,7 +101,6 @@ var newReceiverTests = []struct {
 		"$t = new TestReceiver; echo $t->invalid();",
 		"",
 	},
-
 	{
 		"$t = new TestReceiver; echo ($t->Var) ? 1 : 0;",
 		"1",
@@ -123,18 +119,22 @@ var newReceiverTests = []struct {
 	},
 }
 
-func TestNewReceiver(t *testing.T) {
+func TestReceiverDefine(t *testing.T) {
 	var w bytes.Buffer
 
 	c, _ := e.NewContext()
 	c.Output = &w
 
-	r, err := NewReceiver("TestReceiver", newTestReceiver)
-	if err != nil {
-		t.Fatalf("NewReceiver(): Failed to define method receiver: %s", err)
+	if err := c.Define("TestReceiver", newTestReceiver); err != nil {
+		t.Fatalf("Engine.Define(): Failed to define method receiver: %s", err)
 	}
 
-	for _, tt := range newReceiverTests {
+	// Attempting to define a receiver twice should fail.
+	if err := c.Define("TestReceiver", newTestReceiver); err == nil {
+		t.Fatalf("Engine.Define(): Defining duplicate receiver should fail")
+	}
+
+	for _, tt := range receiverDefineTests {
 		_, err := c.Eval(tt.script)
 		if err != nil {
 			t.Errorf("Context.Eval('%s'): %s", tt.script, err)
@@ -149,17 +149,20 @@ func TestNewReceiver(t *testing.T) {
 		}
 	}
 
-	r.Destroy()
 	c.Destroy()
 }
 
 func TestReceiverDestroy(t *testing.T) {
 	c, _ := e.NewContext()
 	defer c.Destroy()
 
-	r, err := NewReceiver("TestReceiver", newTestReceiver)
-	if err != nil {
-		t.Fatalf("NewReceiver(): Failed to define method receiver: %s", err)
+	if err := c.Define("TestReceiver", newTestReceiver); err != nil {
+		t.Fatalf("Engine.Define(): Failed to define method receiver: %s", err)
+	}
+
+	r := c.receivers["TestReceiver"]
+	if r == nil {
+		t.Fatalf("Receiver.Destroy(): Could not find defined receiver")
 	}
 
 	r.Destroy()