Use reflection instead of constructor functions to create extended stanza structures.
authorChris Jones <christian.jones@sri.com>
Sat, 07 Sep 2013 11:19:29 -0700
changeset 128 8342afcffc92
parent 127 a8f9a0c07fc8
child 129 cccf2b2fe34d
Use reflection instead of constructor functions to create extended stanza structures.
TODO.txt
xmpp/roster.go
xmpp/roster_test.go
xmpp/stream.go
xmpp/structs.go
xmpp/structs_test.go
xmpp/xmpp.go
xmpp/xmpp_test.go
--- a/TODO.txt	Sat Sep 07 10:30:22 2013 -0700
+++ b/TODO.txt	Sat Sep 07 11:19:29 2013 -0700
@@ -1,6 +1,3 @@
-Extension.StanzaHandlers should use reflection, not constructor
-functions.
-
 Review all these *Client receiver methods. They should probably either
 all be receivers, or none.
 
--- a/xmpp/roster.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/roster.go	Sat Sep 07 11:19:29 2013 -0700
@@ -8,6 +8,7 @@
 
 import (
 	"encoding/xml"
+	"reflect"
 )
 
 // Roster query/result
@@ -32,9 +33,9 @@
 
 type Roster struct {
 	Extension
-	get chan []RosterItem
+	get       chan []RosterItem
 	callbacks chan rosterCb
-	toServer chan Stanza
+	toServer  chan Stanza
 }
 
 type rosterClient struct {
@@ -42,23 +43,18 @@
 	rosterUpdate chan<- RosterItem
 }
 
-// Implicitly becomes part of NewClient's extStanza arg.
-func newRosterQuery(name *xml.Name) interface{} {
-	return &RosterQuery{}
-}
-
 func (r *Roster) rosterMgr(upd <-chan Stanza) {
 	roster := make(map[string]RosterItem)
 	waits := make(map[string]func())
 	var snapshot []RosterItem
 	for {
 		select {
-		case stan, ok := <- upd:
+		case stan, ok := <-upd:
 			if !ok {
 				return
 			}
 			hdr := stan.GetHeader()
-			if f := waits[hdr.Id] ; f != nil {
+			if f := waits[hdr.Id]; f != nil {
 				delete(waits, hdr.Id)
 				f()
 			}
@@ -84,7 +80,7 @@
 				snapshot = append(snapshot, ri)
 			}
 		case r.get <- snapshot:
-		case cb := <- r.callbacks:
+		case cb := <-r.callbacks:
 			waits[cb.id] = cb.cb
 		}
 	}
@@ -104,12 +100,12 @@
 		defer close(out)
 		for {
 			select {
-			case stan, ok := <- in:
+			case stan, ok := <-in:
 				if !ok {
 					return
 				}
 				out <- stan
-			case stan := <- r.toServer:
+			case stan := <-r.toServer:
 				out <- stan
 			}
 		}
@@ -119,8 +115,9 @@
 
 func newRosterExt() *Roster {
 	r := Roster{}
-	r.StanzaHandlers = make(map[string]func(*xml.Name) interface{})
-	r.StanzaHandlers[NsRoster] = newRosterQuery
+	r.StanzaHandlers = make(map[xml.Name]reflect.Type)
+	rName := xml.Name{Space: NsRoster, Local: "query"}
+	r.StanzaHandlers[rName] = reflect.TypeOf(RosterQuery{})
 	r.RecvFilter, r.SendFilter = r.makeFilters()
 	r.get = make(chan []RosterItem)
 	r.callbacks = make(chan rosterCb)
--- a/xmpp/roster_test.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/roster_test.go	Sat Sep 07 11:19:29 2013 -0700
@@ -25,7 +25,9 @@
 		NsRoster + `"><item jid="a@b.c"/></query></iq>`
 	iq := Iq{}
 	xml.Unmarshal([]byte(str), &iq)
-	m := map[string]func(*xml.Name) interface{}{NsRoster: newRosterQuery}
+	m := make(map[xml.Name]reflect.Type)
+	name := xml.Name{Space: NsRoster, Local: "query"}
+	m[name] = reflect.TypeOf(RosterQuery{})
 	err := parseExtended(&iq.Header, m)
 	if err != nil {
 		t.Fatalf("parseExtended: %v", err)
--- a/xmpp/stream.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/stream.go	Sat Sep 07 11:19:29 2013 -0700
@@ -20,6 +20,7 @@
 	"io"
 	"math/big"
 	"net"
+	"reflect"
 	"regexp"
 	"strings"
 	"time"
@@ -76,7 +77,7 @@
 }
 
 func readXml(r io.Reader, ch chan<- interface{},
-	extStanza map[string]func(*xml.Name) interface{}) {
+	extStanza map[xml.Name]reflect.Type) {
 	if _, ok := Debug.(*noLog); !ok {
 		pr, pw := io.Pipe()
 		go tee(r, pw, "S: ")
@@ -162,7 +163,7 @@
 	}
 }
 
-func parseExtended(st *Header, extStanza map[string]func(*xml.Name) interface{}) error {
+func parseExtended(st *Header, extStanza map[xml.Name]reflect.Type) error {
 	// Now parse the stanza's innerxml to find the string that we
 	// can unmarshal this nested element from.
 	reader := strings.NewReader(st.Innerxml)
@@ -176,9 +177,8 @@
 			return err
 		}
 		if se, ok := t.(xml.StartElement); ok {
-			if con, ok := extStanza[se.Name.Space]; ok {
-				// Call the indicated constructor.
-				nested := con(&se.Name)
+			if typ, ok := extStanza[se.Name]; ok {
+				nested := reflect.New(typ).Interface()
 
 				// Unmarshal the nested element and
 				// stuff it back into the stanza.
--- a/xmpp/structs.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/structs.go	Sat Sep 07 11:19:29 2013 -0700
@@ -13,6 +13,7 @@
 	"fmt"
 	// BUG(cjyar): Doesn't use stringprep. Could try the implementation at
 	// "code.google.com/p/go-idn/src/stringprep"
+	"reflect"
 	"regexp"
 	"strings"
 )
@@ -269,8 +270,10 @@
 	return string(buf)
 }
 
-var bindExt Extension = Extension{StanzaHandlers: map[string]func(*xml.Name) interface{}{NsBind: newBind}}
+var bindExt Extension = Extension{}
 
-func newBind(name *xml.Name) interface{} {
-	return &bindIq{}
+func init() {
+	bindExt.StanzaHandlers = make(map[xml.Name]reflect.Type)
+	bName := xml.Name{Space: NsBind, Local: "bind"}
+	bindExt.StanzaHandlers[bName] = reflect.TypeOf(bindIq{})
 }
--- a/xmpp/structs_test.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/structs_test.go	Sat Sep 07 11:19:29 2013 -0700
@@ -122,7 +122,7 @@
 	str := `<message to="a@b.c"><body>foo!</body></message>`
 	r := strings.NewReader(str)
 	ch := make(chan interface{})
-	go readXml(r, ch, make(map[string]func(*xml.Name) interface{}))
+	go readXml(r, ch, make(map[xml.Name]reflect.Type))
 	obs := <-ch
 	exp := &Message{XMLName: xml.Name{Local: "message", Space: "jabber:client"},
 		Header: Header{To: "a@b.c", Innerxml: "<body>foo!</body>"},
--- a/xmpp/xmpp.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/xmpp.go	Sat Sep 07 11:19:29 2013 -0700
@@ -18,6 +18,7 @@
 	"fmt"
 	"io"
 	"net"
+	"reflect"
 	"sync"
 )
 
@@ -51,7 +52,7 @@
 	// Maps from an XML namespace to a function which constructs a
 	// structure to hold the contents of stanzas in that
 	// namespace.
-	StanzaHandlers map[string]func(*xml.Name) interface{}
+	StanzaHandlers map[xml.Name]reflect.Type
 	// If non-nil, will be called once to start the filter
 	// running. RecvFilter intercepts incoming messages on their
 	// way from the remote server to the application; SendFilter
@@ -91,7 +92,7 @@
 	// asynchronously as new features are received throughout the
 	// connection process. It should not be updated once
 	// StartSession() returns.
-	Features  *Features
+	Features                     *Features
 	sendFilterAdd, recvFilterAdd chan Filter
 }
 
@@ -141,9 +142,13 @@
 	cl.handlers = make(chan *stanzaHandler, 100)
 	cl.inputControl = make(chan int)
 
-	extStanza := make(map[string]func(*xml.Name) interface{})
+	extStanza := make(map[xml.Name]reflect.Type)
 	for _, ext := range exts {
 		for k, v := range ext.StanzaHandlers {
+			if _, ok := extStanza[k]; !ok {
+				return nil, fmt.Errorf("duplicate handler %s",
+					k)
+			}
 			extStanza[k] = v
 		}
 	}
--- a/xmpp/xmpp_test.go	Sat Sep 07 10:30:22 2013 -0700
+++ b/xmpp/xmpp_test.go	Sat Sep 07 11:19:29 2013 -0700
@@ -17,7 +17,7 @@
 	r := strings.NewReader(`<stream:error><bad-foo xmlns="blah"/>` +
 		`</stream:error>`)
 	ch := make(chan interface{})
-	go readXml(r, ch, make(map[string]func(*xml.Name) interface{}))
+	go readXml(r, ch, make(map[xml.Name]reflect.Type))
 	x := <-ch
 	se, ok := x.(*streamError)
 	if !ok {
@@ -33,7 +33,7 @@
 		`<text xml:lang="en" xmlns="` + NsStreams +
 		`">Error text</text></stream:error>`)
 	ch = make(chan interface{})
-	go readXml(r, ch, make(map[string]func(*xml.Name) interface{}))
+	go readXml(r, ch, make(map[xml.Name]reflect.Type))
 	x = <-ch
 	se, ok = x.(*streamError)
 	if !ok {
@@ -51,7 +51,7 @@
 		`xmlns="` + NsClient + `" xmlns:stream="` + NsStream +
 		`" version="1.0">`)
 	ch := make(chan interface{})
-	go readXml(r, ch, make(map[string]func(*xml.Name) interface{}))
+	go readXml(r, ch, make(map[xml.Name]reflect.Type))
 	x := <-ch
 	ss, ok := x.(*stream)
 	if !ok {