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

Headers improvements #23

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
78 changes: 32 additions & 46 deletions protocol/headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
package protocol

import (
"encoding/json"
"fmt"
"strconv"
"strings"
"time"

"github.com/google/uuid"
)

const (
Expand Down Expand Up @@ -72,15 +75,16 @@ const (
HeaderContentType = "content-type"
)

// Headers represents all Ditto-specific headers along with additional HTTP/etc. headers
// Headers represents all Ditto-specific headers along with additional HTTP/etc. Headers
// that can be applied depending on the transport used.
// For the pre-defined headers, the values are in the row type. The getter methods are provided
// to get the header value in specified type.
//
// The header values in this map should be serialized.
// The provided getter methods returns the header values which is associated with this definition's key.
// See https://www.eclipse.org/ditto/protocol-specification.html
type Headers map[string]interface{}

// CorrelationID returns the 'correlation-id' header value if it is presented.
// If the header value is not presented, the CorrelationID returns empty string.
// If the header value is not presented, the 'correlation-id' header value will be generated in UUID format.
//
// If there are two headers differing only in capitalization CorrelationID returns the first value.
Copy link
Contributor

Choose a reason for hiding this comment

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

If there are more than one headers differing only in capitalization CorrelationID returns the first met value.

// To use the provided key to get the value, access the map directly.
Expand All @@ -90,10 +94,10 @@ func (h Headers) CorrelationID() string {
return v.(string)
}
}
return ""
return uuid.New().String()
Copy link
Contributor

Choose a reason for hiding this comment

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

If initially missing, on every invocation of CorrelationID() a newly generated id would be returned.
Also if there are more than one headers differing only in capitalization, should we make sure that the one that is initially returned(the first met one) would be returned on every next invocation as well?

}

// Timeout returns the 'timeout' header value if it is presented
// Timeout returns the 'timeout' header value if it is presented.
// The default and maximum value is duration of 60 seconds.
// If the header value is not presented, the Timout returns the default value.
//
Expand Down Expand Up @@ -134,7 +138,7 @@ func parseTimeout(timeout string) (time.Duration, error) {
t = time.Duration(i) * time.Second
}
}
if t >= 0 && t < time.Hour {
if t >= 0 && t <= 60*time.Second {
return t, nil
}
}
Expand Down Expand Up @@ -284,7 +288,7 @@ func (h Headers) ReplyTo() string {
}

// Version returns the 'version' header value if it is presented.
// If the header value is not presented, the Version returns 0.
// If the header value is not presented, the Version returns 2.
//
// If there are two headers differing only in capitalization, the Version returns the first value.
// To use the provided key to get the value, access the map directly.
Expand All @@ -294,7 +298,7 @@ func (h Headers) Version() int64 {
return v.(int64)
}
}
return 0
return 2
}

// ContentType returns the 'content-type' header value if it is presented.
Expand Down Expand Up @@ -325,44 +329,26 @@ func (h Headers) Generic(id string) interface{} {
return nil
}

// // MarshalJSON marshels Headers.
// func (h *Headers) MarshalJSON() ([]byte, error) {
// // TODO validation
// // convert - timeout - ditto timeout string
// // error for invalid values
// return json.Marshal(h.Values)
// }

// UnmarshalJSON unmarshels Headers.
// func (h *Headers) UnmarshalJSON(data []byte) error {
// var m map[string]interface{}

// if err := json.Unmarshal(data, &m); err != nil {
// return err
// }

// for k := range m {
// // TODO for all headers
// // error for ivalid values
// if strings.EqualFold(k, HeaderTimeout) && m[k] != nil {
// m[k] = parseTimeout(m[k].(string))
// }
// }

// h.Values = m

// return nil
// }

// UnmarshalJSON unmarshels Headers.
// func (h *Headers) UnmarshalJSON(data []byte) error {
// temp := make(map[string]interface{})
// if err := json.Unmarshal(data, &temp); err != nil {
// return err
// }
// *h = temp
// return nil
// }
//
// The header names are case-insensitive and case-preserving.
// If there is the same header name as the provided and the difference is
// in capitalization the new header name will be set.
func (h *Headers) UnmarshalJSON(data []byte) error {
temp := make(map[string]interface{})
if err := json.Unmarshal(data, &temp); err != nil {
return err
}
for k := range temp {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this cleaning needed? In the generic case the initial headers are empty.

for k1 := range *h {
if strings.EqualFold(k, k1) {
delete(*h, k1)
}
}
}
*h = temp
return nil
}

// With sets new Headers to the existing.
func (h Headers) With(opts ...HeaderOpt) Headers {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove it.

Expand Down
10 changes: 10 additions & 0 deletions protocol/headers_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package protocol

import (
"strconv"
"strings"
"time"
)

Expand Down Expand Up @@ -188,8 +189,17 @@ func WithContentType(contentType string) HeaderOpt {
}

// WithGeneric sets the value of the provided key header.
//
// The header names are case-insensitive and case-preserving.
// If there is the same header name as the provided and the difference is
// in capitalization the new header name will be set.
func WithGeneric(headerID string, value interface{}) HeaderOpt {
return func(headers Headers) error {
for k := range headers {
if strings.EqualFold(k, headerID) {
delete(headers, k)
Copy link
Contributor

Choose a reason for hiding this comment

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

The user might want to add several different headers with keys only differing in the capitalization. In this case the user should directly use the exported map?

}
}
headers[headerID] = value
return nil
}
Expand Down
23 changes: 19 additions & 4 deletions protocol/headers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ func TestHeadersCorrelationID(t *testing.T) {
},
"test_without_correlation_id": {
testHeader: Headers{},
want: "",
},

"test_empty_correlation_id": {
testHeader: Headers{HeaderCorrelationID: ""},
want: "",
Expand All @@ -42,7 +40,11 @@ func TestHeadersCorrelationID(t *testing.T) {
for testName, testCase := range tests {
t.Run(testName, func(t *testing.T) {
got := testCase.testHeader.CorrelationID()
internal.AssertEqual(t, testCase.want, got)
if testCase.testHeader[HeaderCorrelationID] == nil {
internal.AssertNotNil(t, got)
} else {
internal.AssertEqual(t, testCase.want, got)
}
})
}
}
Expand Down Expand Up @@ -367,7 +369,7 @@ func TestHeadersVersion(t *testing.T) {
},
"test_without_version": {
testHeader: Headers{},
want: 0,
want: 2,
},
}

Expand Down Expand Up @@ -509,3 +511,16 @@ func TestCaseInsensitiveKey(t *testing.T) {
json.Unmarshal([]byte(data), &envelope.Headers)
internal.AssertEqual(t, "correlation-id-3", envelope.Headers["correlation-iD"])
}

func TestCasePreservedKey(t *testing.T) {
headers := Headers{HeaderCorrelationID: "correlation-id-1"}

data := `{
"correlation-iD":"correlation-id-2"
}`

want := Headers{"correlation-iD": "correlation-id-2"}

headers.UnmarshalJSON([]byte(data))
internal.AssertEqual(t, want, headers)
}