Preserve whether an instance's ports are enabled

When transcribing the fargo.Instance struct to and from both JSON and XML, indicate and retain whether each port is enabled or not. Note that in the Java Eureka library, by default the insecure port is enabled and the secure port is disabled. Here the zero value of a fargo.Instance has the insecure port disabled as well. While it would be possible to invert the sense of the corresponding field to indicate whether the insecure port is disabled, so that the zero value of false matches the Java library's behavior, the resulting asymmetry with the secure port's field is too awkward. Instead, require registrants to enable the ports explicitly. This change removes the exported fargo.Port type and several exported fields of the fargo.Instance type: XMLName PortJ SecurePortJ None of these were exposed deliberately for use by callers; they were all exposed only as accidental consequences of the JSON and XML marshalling library's idiosyncrasies.
jonhenrik13 · Jan 26, 2017 · 9352b64 · 9352b64
1 parent e8c3b0f
commit 9352b64
Show file tree

Hide file tree

Showing 4 changed files with 262 additions and 44 deletions.
diff --git a/marshal.go b/marshal.go
@@ -76,22 +76,32 @@ func (a *Application) UnmarshalJSON(b []byte) error {
 	return err
 }
 
-type instance Instance
+// jsonFormatPort describes an instance's network port, including whether its registrant considers
+// the port to be enabled or disabled.
+//
+// Example JSON encoding:
+//
+//   "port":{"@enabled":"true", "$":"7101"}
+//
+// Note that later versions of Eureka write the port number as a JSON number rather than as a
+// decimal-formatted string.
+type jsonFormatPort struct {
+	Number  string `json:"$"`
+	Enabled string `json:"@enabled"`
+}
 
-// UnmarshalJSON is a custom JSON unmarshaler for Instance to deal with the
-// different Port encodings between XML and JSON. Here we copy the values from the JSON
-// Port struct into the simple XML int field.
-func (i *Instance) UnmarshalJSON(b []byte) error {
-	var err error
-	var ii instance
-	if err = json.Unmarshal(b, &ii); err == nil {
-		marshalLog.Debug("Instance.UnmarshalJSON ii:%+v\n", ii)
-		*i = Instance(ii)
-		i.Port = parsePort(ii.PortJ.Number)
-		i.SecurePort = parsePort(ii.SecurePortJ.Number)
-		return nil
+func boolAsString(b bool) string {
+	if b {
+		return "true"
+	}
+	return "false"
+}
+
+func makeJSONFormatPort(port int, enabled bool) jsonFormatPort {
+	return jsonFormatPort{
+		strconv.Itoa(port),
+		boolAsString(enabled),
 	}
-	return err
 }
 
 func parsePort(s string) int {
@@ -102,6 +112,100 @@ func parsePort(s string) int {
 	return n
 }
 
+func stringAsBool(s string) bool {
+	return s == "true"
+}
+
+// UnmarshalJSON is a custom JSON unmarshaler for Instance, transcribing the two composite port
+// specifications up to top-level fields.
+func (i *Instance) UnmarshalJSON(b []byte) error {
+	type instance Instance
+	aux := struct {
+		*instance
+		Port       jsonFormatPort `json:"port"`
+		SecurePort jsonFormatPort `json:"securePort"`
+	}{
+		instance: (*instance)(i),
+	}
+	if err := json.Unmarshal(b, &aux); err != nil {
+		return err
+	}
+	i.Port = parsePort(aux.Port.Number)
+	i.PortEnabled = stringAsBool(aux.Port.Enabled)
+	i.SecurePort = parsePort(aux.SecurePort.Number)
+	i.SecurePortEnabled = stringAsBool(aux.SecurePort.Enabled)
+	return nil
+}
+
+// MarshalJSON is a custom JSON marshaler for Instance, adapting the top-level raw port values to
+// the composite port specifications.
+func (i *Instance) MarshalJSON() ([]byte, error) {
+	type instance Instance
+	aux := struct {
+		*instance
+		Port       jsonFormatPort `json:"port"`
+		SecurePort jsonFormatPort `json:"securePort"`
+	}{
+		(*instance)(i),
+		makeJSONFormatPort(i.Port, i.PortEnabled),
+		makeJSONFormatPort(i.SecurePort, i.SecurePortEnabled),
+	}
+	return json.Marshal(&aux)
+}
+
+// xmlFormatPort describes an instance's network port, including whether its registrant considers
+// the port to be enabled or disabled.
+//
+// Example XML encoding:
+//
+//     <port enabled="true">7101</port>
+type xmlFormatPort struct {
+	Number  int  `xml:",chardata"`
+	Enabled bool `xml:"enabled,attr"`
+}
+
+// UnmarshalXML is a custom XML unmarshaler for Instance, transcribing the two composite port
+// specifications up to top-level fields.
+func (i *Instance) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
+	type instance Instance
+	aux := struct {
+		*instance
+		Port       xmlFormatPort `xml:"port"`
+		SecurePort xmlFormatPort `xml:"securePort"`
+	}{
+		instance: (*instance)(i),
+	}
+	if err := d.DecodeElement(&aux, &start); err != nil {
+		return err
+	}
+	i.Port = aux.Port.Number
+	i.PortEnabled = aux.Port.Enabled
+	i.SecurePort = aux.SecurePort.Number
+	i.SecurePortEnabled = aux.SecurePort.Enabled
+	return nil
+}
+
+// startLocalName creates a start-tag of an XML element with the given local name and no namespace name.
+func startLocalName(local string) xml.StartElement {
+	return xml.StartElement{Name: xml.Name{Space: "", Local: local}}
+}
+
+// MarshalXML is a custom XML marshaler for Instance, adapting the top-level raw port values to
+// the composite port specifications.
+func (i *Instance) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
+	type instance Instance
+	aux := struct {
+		*instance
+		Port       xmlFormatPort `xml:"port"`
+		SecurePort xmlFormatPort `xml:"securePort"`
+	}{
+		instance:   (*instance)(i),
+		Port:       xmlFormatPort{i.Port, i.PortEnabled},
+		SecurePort: xmlFormatPort{i.SecurePort, i.SecurePortEnabled},
+	}
+	return e.EncodeElement(&aux, startLocalName("instance"))
+}
+
 // UnmarshalJSON is a custom JSON unmarshaler for InstanceMetadata to handle squirreling away
 // the raw JSON for later parsing.
 func (i *InstanceMetadata) UnmarshalJSON(b []byte) error {
@@ -123,11 +227,6 @@ func (i *InstanceMetadata) MarshalJSON() ([]byte, error) {
 	return i.Raw, nil
 }
 
-// startLocalName creates a start-tag of an XML element with the given local name and no namespace name.
-func startLocalName(local string) xml.StartElement {
-	return xml.StartElement{Name: xml.Name{Space: "", Local: local}}
-}
-
 // MarshalXML is a custom XML marshaler for InstanceMetadata.
 func (i InstanceMetadata) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
 	tokens := []xml.Token{start}

diff --git a/net.go b/net.go
@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"math/rand"
 	"net/http"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -658,8 +657,6 @@ func (e *EurekaConnection) ReregisterInstance(ins *Instance) error {
 	var out []byte
 	var err error
 	if e.UseJson {
-		ins.PortJ.Number = strconv.Itoa(ins.Port)
-		ins.SecurePortJ.Number = strconv.Itoa(ins.SecurePort)
 		out, err = e.marshal(&RegisterInstanceJson{ins})
 	} else {
 		out, err = e.marshal(ins)

diff --git a/struct.go b/struct.go
@@ -40,7 +40,7 @@ type GetAppsResponse struct {
 	VersionsDelta int            `xml:"versions__delta" json:"versions__delta"`
 }
 
-// Application deserializeable from Eureka JSON.
+// GetAppResponseJson wraps an Application for deserializing from Eureka JSON.
 type GetAppResponseJson struct {
 	Application Application `json:"application"`
 }
@@ -74,22 +74,21 @@ type RegisterInstanceJson struct {
 	Instance *Instance `json:"instance"`
 }
 
-// Instance [de]serializeable [to|from] Eureka XML.
+// Instance [de]serializeable [to|from] Eureka [XML|JSON].
 type Instance struct {
-	XMLName          struct{} `xml:"instance" json:"-"`
-	HostName         string   `xml:"hostName" json:"hostName"`
-	App              string   `xml:"app" json:"app"`
-	IPAddr           string   `xml:"ipAddr" json:"ipAddr"`
-	VipAddress       string   `xml:"vipAddress" json:"vipAddress"`
-	SecureVipAddress string   `xml:"secureVipAddress" json:"secureVipAddress"`
+	HostName         string `xml:"hostName" json:"hostName"`
+	App              string `xml:"app" json:"app"`
+	IPAddr           string `xml:"ipAddr" json:"ipAddr"`
+	VipAddress       string `xml:"vipAddress" json:"vipAddress"`
+	SecureVipAddress string `xml:"secureVipAddress" json:"secureVipAddress"`
 
 	Status           StatusType `xml:"status" json:"status"`
 	Overriddenstatus StatusType `xml:"overriddenstatus" json:"overriddenstatus"`
 
-	Port        int  `xml:"port" json:"-"`
-	PortJ       Port `json:"port" xml:"-"`
-	SecurePort  int  `xml:"securePort" json:"-"`
-	SecurePortJ Port `json:"securePort" xml:"-"`
+	Port              int  `xml:"-" json:"-"`
+	PortEnabled       bool `xml:"-" json:"-"`
+	SecurePort        int  `xml:"-" json:"-"`
+	SecurePortEnabled bool `xml:"-" json:"-"`
 
 	HomePageUrl    string `xml:"homePageUrl" json:"homePageUrl"`
 	StatusPageUrl  string `xml:"statusPageUrl" json:"statusPageUrl"`
@@ -104,14 +103,6 @@ type Instance struct {
 	UniqueID func(i Instance) string `xml:"-" json:"-"`
 }
 
-// Port struct used for JSON [un]marshaling only.
-// An example:
-// 	"port":{"@enabled":"true", "$":"7101"}
-type Port struct {
-	Number  string `json:"$"`
-	Enabled string `json:"@enabled"`
-}
-
 // InstanceMetadata represents the eureka metadata, which is arbitrary XML.
 // See metadata.go for more info.
 type InstanceMetadata struct {

diff --git a/tests/marshal_test.go b/tests/marshal_test.go
@@ -45,9 +45,140 @@ func TestJsonMarshal(t *testing.T) {
 	}
 }
 
+func portsEqual(actual, expected *fargo.Instance) {
+	Convey("For the insecure port", func() {
+		So(actual.Port, ShouldEqual, expected.Port)
+		So(actual.PortEnabled, ShouldEqual, expected.PortEnabled)
+
+		Convey("For the secure port", func() {
+			So(actual.SecurePort, ShouldEqual, expected.SecurePort)
+			So(actual.SecurePortEnabled, ShouldEqual, expected.SecurePortEnabled)
+		})
+	})
+}
+
+func jsonEncodedInstanceHasPortsEqualTo(b []byte, expected *fargo.Instance) {
+	Convey("And reading them back should yield the equivalent value", func() {
+		var decoded fargo.Instance
+		err := json.Unmarshal(b, &decoded)
+		So(err, ShouldBeNil)
+		portsEqual(&decoded, expected)
+	})
+}
+
+func xmlEncodedInstanceHasPortsEqualTo(b []byte, expected *fargo.Instance) {
+	Convey("And reading them back should yield the equivalent value", func() {
+		var decoded fargo.Instance
+		err := xml.Unmarshal(b, &decoded)
+		So(err, ShouldBeNil)
+		portsEqual(&decoded, expected)
+	})
+}
+
+func TestPortsMarshal(t *testing.T) {
+	Convey("Given an Instance with only the insecure port enabled", t, func() {
+		ins := fargo.Instance{
+			Port:        80,
+			PortEnabled: true,
+		}
+
+		Convey("When the ports are marshalled as JSON", func() {
+			b, err := json.Marshal(&ins)
+
+			Convey("The marshalled JSON should have these values", func() {
+				So(err, ShouldBeNil)
+				s := string(b)
+				So(s, ShouldContainSubstring, `,"port":{"$":"80","@enabled":"true"}`)
+				So(s, ShouldContainSubstring, `,"securePort":{"$":"0","@enabled":"false"}`)
+
+				jsonEncodedInstanceHasPortsEqualTo(b, &ins)
+			})
+		})
+
+		Convey("When the ports are marshalled as XML", func() {
+			b, err := xml.Marshal(&ins)
+
+			Convey("The marshalled XML should have these values", func() {
+				So(err, ShouldBeNil)
+				s := string(b)
+				So(s, ShouldContainSubstring, `<port enabled="true">80</port>`)
+				So(s, ShouldContainSubstring, `<securePort enabled="false">0</securePort>`)
+
+				xmlEncodedInstanceHasPortsEqualTo(b, &ins)
+			})
+		})
+	})
+	Convey("Given an Instance with only the secure port enabled", t, func() {
+		ins := fargo.Instance{
+			SecurePort:        443,
+			SecurePortEnabled: true,
+		}
+
+		Convey("When the ports are marshalled as JSON", func() {
+			b, err := json.Marshal(&ins)
+
+			Convey("The marshalled JSON should have these values", func() {
+				So(err, ShouldBeNil)
+				s := string(b)
+				So(s, ShouldContainSubstring, `,"port":{"$":"0","@enabled":"false"}`)
+				So(s, ShouldContainSubstring, `,"securePort":{"$":"443","@enabled":"true"}`)
+
+				jsonEncodedInstanceHasPortsEqualTo(b, &ins)
+			})
+		})
+
+		Convey("When the ports are marshalled as XML", func() {
+			b, err := xml.Marshal(&ins)
+
+			Convey("The marshalled XML should have these values", func() {
+				So(err, ShouldBeNil)
+				s := string(b)
+				So(s, ShouldContainSubstring, `<port enabled="false">0</port>`)
+				So(s, ShouldContainSubstring, `<securePort enabled="true">443</securePort>`)
+
+				xmlEncodedInstanceHasPortsEqualTo(b, &ins)
+			})
+		})
+	})
+	Convey("Given an Instance with only the both ports enabled", t, func() {
+		ins := fargo.Instance{
+			Port:              80,
+			PortEnabled:       true,
+			SecurePort:        443,
+			SecurePortEnabled: true,
+		}
+
+		Convey("When the ports are marshalled as JSON", func() {
+			b, err := json.Marshal(&ins)
+
+			Convey("The marshalled JSON should have these values", func() {
+				So(err, ShouldBeNil)
+				s := string(b)
+				So(s, ShouldContainSubstring, `,"port":{"$":"80","@enabled":"true"}`)
+				So(s, ShouldContainSubstring, `,"securePort":{"$":"443","@enabled":"true"}`)
+
+				jsonEncodedInstanceHasPortsEqualTo(b, &ins)
+			})
+		})
+
+		Convey("When the ports are marshalled as XML", func() {
+			b, err := xml.Marshal(&ins)
+
+			Convey("The marshalled XML should have these values", func() {
+				So(err, ShouldBeNil)
+				s := string(b)
+				So(s, ShouldContainSubstring, `<port enabled="true">80</port>`)
+				So(s, ShouldContainSubstring, `<securePort enabled="true">443</securePort>`)
+
+				xmlEncodedInstanceHasPortsEqualTo(b, &ins)
+			})
+		})
+	})
+}
+
 func TestMetadataMarshal(t *testing.T) {
 	Convey("Given an Instance with metadata", t, func() {
-		ins := &fargo.Instance{}
+		ins := fargo.Instance{}
 		ins.SetMetadataString("key1", "value1")
 		ins.SetMetadataString("key2", "value2")
 
@@ -75,7 +206,7 @@ func TestMetadataMarshal(t *testing.T) {
 
 func TestDataCenterInfoMarshal(t *testing.T) {
 	Convey("Given an Instance situated in a data center", t, func() {
-		ins := &fargo.Instance{}
+		ins := fargo.Instance{}
 
 		Convey("When the data center name is \"Amazon\"", func() {
 			ins.DataCenterInfo.Name = fargo.Amazon