Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve an instance's enabled status for its two ports #59

Merged
merged 5 commits into from
Mar 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
286 changes: 228 additions & 58 deletions marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,66 @@ package fargo
import (
"encoding/json"
"encoding/xml"
"fmt"
"io"
"strconv"
)

// Temporary structs used for GetAppsResponse unmarshalling
type getAppsResponseArray GetAppsResponse
type getAppsResponseSingle struct {
Application *Application `json:"application"`
AppsHashcode string `json:"apps__hashcode"`
VersionsDelta int `json:"versions__delta"`
func intFromJSONNumberOrString(jv interface{}, description string) (int, error) {
switch v := jv.(type) {
case float64:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this float64 and not int? Have you checked it's coming as a fload? Any thoughts of adding inte there as well?

Copy link
Contributor Author

@seh seh Feb 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the documentation for json.Unmarshal, numbers coming from JSON into Go are always of type float64—barring use of json.(*Decoder).UseNumber. (That's implemented here.) As it stands, fargo does not set this option on a Decoder when decoding a fargo.GetAppsResponse.

return int(v), nil
case string:
n, err := strconv.Atoi(v)
if err != nil {
return 0, err
}
return n, nil
default:
return 0, fmt.Errorf("unexpected %s: %[2]v (type %[2]T)", description, jv)
}
}

// UnmarshalJSON is a custom JSON unmarshaler for GetAppsResponse to deal with
// sometimes non-wrapped Application arrays when there is only a single Application item.
func (r *GetAppsResponse) UnmarshalJSON(b []byte) error {
marshalLog.Debugf("GetAppsResponse.UnmarshalJSON b:%s\n", string(b))
var err error
resolveDelta := func(d interface{}) (int, error) {
return intFromJSONNumberOrString(d, "versions delta")
}

// Normal array case
var ra getAppsResponseArray
if err = json.Unmarshal(b, &ra); err == nil {
marshalLog.Debug("GetAppsResponse.UnmarshalJSON ra:%+v\n", ra)
*r = GetAppsResponse(ra)
return nil
type getAppsResponse GetAppsResponse
auxArray := struct {
*getAppsResponse
VersionsDelta interface{} `json:"versions__delta"`
}{
getAppsResponse: (*getAppsResponse)(r),
}
var err error
if err = json.Unmarshal(b, &auxArray); err == nil {
marshalLog.Debugf("GetAppsResponse.UnmarshalJSON array:%+v\n", auxArray)
r.VersionsDelta, err = resolveDelta(auxArray.VersionsDelta)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little bit confused here. Previously we were unmarshaling here with *r = GetAppsResponse(ra) but now it seams to me that we are only unmarshaling VersionsDelta here?
Also minor, what does the auxArray stand for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note line 42, where we populate the "auxArray" variable's "getAppsResponse" (anonymous|embedded) field with variable "r." When we supply "auxArray' to json.Unmarshal at line 45, it will write directly to a "r" (dereferencing the pointer as necessary), which is analogous to the previous assignment you recall. It will write to "r" and also write into the auxiliary struct's "VersionsDelta" field, which we then convert and assign back into "r"'s "VersionsDelta" field. The outer struct's field (of type interface{}) shadows the field with the same name in the embedded "getAppsResponse" field (of type int).

Here the variable name "auxArray" means "auxiliary structure for the "array of applications" case, versus the "single application" case handled at line 52. The variable name "aux" comes up frequently with this encoding or decoding technique in Go, where one introduces some separate, temporary, and only locally significant struct to handle the finicky details that Go's struct tags can't accommodate.

return err
}

// Bogus non-wrapped case
var rs getAppsResponseSingle
if err = json.Unmarshal(b, &rs); err == nil {
marshalLog.Debug("GetAppsResponse.UnmarshalJSON rs:%+v\n", rs)
r.Applications = make([]*Application, 1, 1)
r.Applications[0] = rs.Application
r.AppsHashcode = rs.AppsHashcode
r.VersionsDelta = rs.VersionsDelta
return nil
auxSingle := struct {
Application *Application `json:"application"`
AppsHashcode string `json:"apps__hashcode"`
VersionsDelta interface{} `json:"versions__delta"`
}{}
if err := json.Unmarshal(b, &auxSingle); err != nil {
return err
}
return err
marshalLog.Debugf("GetAppsResponse.UnmarshalJSON single:%+v\n", auxSingle)
if r.VersionsDelta, err = resolveDelta(auxSingle.VersionsDelta); err != nil {
return err
}
r.Applications = make([]*Application, 1, 1)
r.Applications[0] = auxSingle.Application
r.AppsHashcode = auxSingle.AppsHashcode
return nil
}

// Temporary structs used for Application unmarshalling
Expand All @@ -59,15 +83,15 @@ func (a *Application) UnmarshalJSON(b []byte) error {
// Normal array case
var aa applicationArray
if err = json.Unmarshal(b, &aa); err == nil {
marshalLog.Debug("Application.UnmarshalJSON aa:%+v\n", aa)
marshalLog.Debugf("Application.UnmarshalJSON aa:%+v\n", aa)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for finding that ⭐️

*a = Application(aa)
return nil
}

// Bogus non-wrapped case
var as applicationSingle
if err = json.Unmarshal(b, &as); err == nil {
marshalLog.Debug("Application.UnmarshalJSON as:%+v\n", as)
marshalLog.Debugf("Application.UnmarshalJSON as:%+v\n", as)
a.Name = as.Name
a.Instances = make([]*Instance, 1, 1)
a.Instances[0] = as.Instance
Expand All @@ -76,30 +100,140 @@ func (a *Application) UnmarshalJSON(b []byte) error {
return err
}

type instance Instance
func stringAsBool(s string) bool {
return s == "true"
}

// 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.
// 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 {
// Preclude recursive calls to MarshalJSON.
type instance Instance
// inboundJSONFormatPort describes an instance's network port, including whether its registrant
// considers the port to be enabled or disabled.
//
// Example JSON encoding:
//
// Eureka versions 1.2.1 and prior:
// "port":{"@enabled":"true", "$":"7101"}
//
// Eureka version 1.2.2 and later:
// "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. We accept it as either an integer or a string. Strangely, the
// "@enabled" field remains a string.
type inboundJSONFormatPort struct {
Number interface{} `json:"$"`
Enabled bool `json:"@enabled,string"`
}
aux := struct {
*instance
Port inboundJSONFormatPort `json:"port"`
SecurePort inboundJSONFormatPort `json:"securePort"`
}{
instance: (*instance)(i),
}
if err := json.Unmarshal(b, &aux); err != nil {
return err
}
resolvePort := func(port interface{}) (int, error) {
return intFromJSONNumberOrString(port, "port number")
}
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
if i.Port, err = resolvePort(aux.Port.Number); err != nil {
return err
}
return err
i.PortEnabled = aux.Port.Enabled
if i.SecurePort, err = resolvePort(aux.SecurePort.Number); err != nil {
return err
}
i.SecurePortEnabled = aux.SecurePort.Enabled
return nil
}

func parsePort(s string) int {
n, err := strconv.Atoi(s)
if err != nil {
log.Warningf("Invalid port error: %s", err.Error())
// 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) {
// Preclude recursive calls to MarshalJSON.
type instance Instance
// outboundJSONFormatPort 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. We emit the port number as a string, not knowing the Eureka
// server's version. Strangely, the "@enabled" field remains a string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thinking exercise. How hard will it be to detect eureka version and serve what's the most appropriate version of date depending on version. Will that be even cleaner? Are we worried that eureka might stop accepting old version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server doesn't send anything indicative of its version—not even in an HTTP header. If we had already received some responses from a server and been able to inspect the fields that differ, we could then remember which "style" or "version" of the encoding we've seen the server use and then mimic that in our future requests. There's still that problem of how our client starts that conversation safely.

As for whether Eureka will stop accepting the old version, I think they'd have to introduce some new resource paths in order to be fair to all the possible clients out there of various versions. My impression is that their handling of the encoding here has been haphazard; it came down to whatever XStream happened to do, and the Java clients could still talk well enough with the servers, so it doesn't appear that they cared enough to document or even prevent these format changes. I don't think they were deliberate.

type outboundJSONFormatPort struct {
Number int `json:"$,string"`
Enabled bool `json:"@enabled,string"`
}
aux := struct {
*instance
Port outboundJSONFormatPort `json:"port"`
SecurePort outboundJSONFormatPort `json:"securePort"`
}{
(*instance)(i),
outboundJSONFormatPort{i.Port, i.PortEnabled},
outboundJSONFormatPort{i.SecurePort, i.SecurePortEnabled},
}
return n
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
Expand All @@ -123,11 +257,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}
Expand Down Expand Up @@ -191,7 +320,7 @@ func (m metadataMap) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error
return nil
}

func metadataValue(i DataCenterInfo) interface{} {
func metadataValue(i *DataCenterInfo) interface{} {
if i.Name == Amazon {
return i.Metadata
}
Expand All @@ -205,7 +334,7 @@ var (

// MarshalXML is a custom XML marshaler for DataCenterInfo, writing either Metadata or AlternateMetadata
// depending on the type of data center indicated by the Name.
func (i DataCenterInfo) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
func (i *DataCenterInfo) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
if err := e.EncodeToken(start); err != nil {
return err
}
Expand All @@ -222,6 +351,7 @@ func (i DataCenterInfo) MarshalXML(e *xml.Encoder, start xml.StartElement) error

type preliminaryDataCenterInfo struct {
Name string `xml:"name" json:"name"`
Class string `xml:"-" json:"@class"`
Metadata metadataMap `xml:"metadata" json:"metadata"`
}

Expand All @@ -247,8 +377,9 @@ func populateAmazonMetadata(dst *AmazonMetadataType, src map[string]string) {
bindValue(&dst.InstanceType, src, "instance-type")
}

func adaptDataCenterInfo(dst *DataCenterInfo, src preliminaryDataCenterInfo) {
func adaptDataCenterInfo(dst *DataCenterInfo, src *preliminaryDataCenterInfo) {
dst.Name = src.Name
dst.Class = src.Class
if src.Name == Amazon {
populateAmazonMetadata(&dst.Metadata, src.Metadata)
} else {
Expand All @@ -265,37 +396,76 @@ func (i *DataCenterInfo) UnmarshalXML(d *xml.Decoder, start xml.StartElement) er
if err := d.DecodeElement(&p, &start); err != nil {
return err
}
adaptDataCenterInfo(i, p)
adaptDataCenterInfo(i, &p)
return nil
}

// MarshalJSON is a custom JSON marshaler for DataCenterInfo, writing either Metadata or AlternateMetadata
// depending on the type of data center indicated by the Name.
func (i DataCenterInfo) MarshalJSON() ([]byte, error) {
func (i *DataCenterInfo) MarshalJSON() ([]byte, error) {
type named struct {
Name string `json:"name"`
Name string `json:"name"`
Class string `json:"@class"`
}
if i.Name == Amazon {
return json.Marshal(struct {
named
Metadata AmazonMetadataType `json:"metadata"`
}{named{i.Name}, i.Metadata})
}{
named{i.Name, "com.netflix.appinfo.AmazonInfo"},
i.Metadata,
})
}
class := "com.netflix.appinfo.MyDataCenterInfo"
if i.Name != MyOwn {
class = i.Class
}
return json.Marshal(struct {
named
Metadata map[string]string `json:"metadata"`
}{named{i.Name}, i.AlternateMetadata})
Metadata map[string]string `json:"metadata,omitempty"`
}{
named{i.Name, class},
i.AlternateMetadata,
})
}

func jsonValueAsString(i interface{}) string {
switch v := i.(type) {
case string:
return v
case float64:
return fmt.Sprintf("%.f", v)
case bool:
return strconv.FormatBool(v)
case []interface{}, map[string]interface{}:
// Don't bother trying to decode these.
return ""
case nil:
return ""
default:
panic("type of unexpected value")
}
}

// UnmarshalJSON is a custom JSON unmarshaler for DataCenterInfo, populating either Metadata or AlternateMetadata
// depending on the type of data center indicated by the Name.
func (i *DataCenterInfo) UnmarshalJSON(b []byte) error {
p := preliminaryDataCenterInfo{
Metadata: make(map[string]string, 11),
// The Eureka server will mistakenly convert metadata values that look like numbers to JSON numbers.
// Convert them back to strings.
aux := struct {
*preliminaryDataCenterInfo
PreliminaryMetadata map[string]interface{} `json:"metadata"`
}{
PreliminaryMetadata: make(map[string]interface{}, 11),
}
if err := json.Unmarshal(b, &p); err != nil {
if err := json.Unmarshal(b, &aux); err != nil {
return err
}
adaptDataCenterInfo(i, p)
metadata := make(map[string]string, len(aux.PreliminaryMetadata))
for k, v := range aux.PreliminaryMetadata {
metadata[k] = jsonValueAsString(v)
}
aux.Metadata = metadata
adaptDataCenterInfo(i, aux.preliminaryDataCenterInfo)
return nil
}
Loading