-
Notifications
You must be signed in to change notification settings - Fork 219
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
Enable a binding.Message to be consumed more times #282
Changes from all commits
9a5b313
a9059df
1719e9c
dfae835
72e49d2
3ea5b24
f943130
4d43686
4f92c8b
4019329
19bb052
9b62bff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package binding | ||
|
||
import "sync/atomic" | ||
|
||
type acksMessage struct { | ||
Message | ||
requiredAcks int32 | ||
} | ||
|
||
func (m *acksMessage) Finish(err error) error { | ||
remainingAcks := atomic.AddInt32(&m.requiredAcks, -1) | ||
if remainingAcks == 0 { | ||
return m.Message.Finish(err) | ||
} | ||
return nil | ||
} | ||
|
||
// WithAcksBeforeFinish returns a wrapper for m that calls m.Finish() | ||
// only after the specified number of acks are received. | ||
// Use it when you need to route a Message to more Sender instances | ||
func WithAcksBeforeFinish(m Message, requiredAcks int) Message { | ||
return &acksMessage{Message: m, requiredAcks: int32(requiredAcks)} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
package binding | ||
|
||
import ( | ||
"context" | ||
"net/url" | ||
"sync" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
|
||
cloudevents "github.com/cloudevents/sdk-go" | ||
"github.com/cloudevents/sdk-go/pkg/cloudevents/types" | ||
) | ||
|
||
func TestWithAcksBeforeFinish(t *testing.T) { | ||
var testEvent = cloudevents.Event{ | ||
Data: []byte(`"data"`), | ||
DataEncoded: true, | ||
Context: cloudevents.EventContextV1{ | ||
DataContentType: cloudevents.StringOfApplicationJSON(), | ||
Source: types.URIRef{URL: url.URL{Path: "source"}}, | ||
ID: "id", | ||
Type: "type"}.AsV1(), | ||
} | ||
|
||
finishCalled := false | ||
finishMessage := WithFinish(EventMessage(testEvent), func(err error) { | ||
finishCalled = true | ||
}) | ||
|
||
wg := sync.WaitGroup{} | ||
|
||
messageToTest := WithAcksBeforeFinish(finishMessage, 1000) | ||
for i := 0; i < 1000; i++ { | ||
wg.Add(1) | ||
go func(m Message) { | ||
ch := make(chan Message, 1) | ||
assert.NoError(t, ChanSender(ch).Send(context.Background(), m)) | ||
<-ch | ||
wg.Done() | ||
}(messageToTest) | ||
} | ||
|
||
wg.Wait() | ||
assert.True(t, finishCalled) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ import ( | |
"context" | ||
"encoding/json" | ||
"io" | ||
"io/ioutil" | ||
|
||
"github.com/cloudevents/sdk-go/pkg/binding" | ||
"github.com/cloudevents/sdk-go/pkg/binding/format" | ||
|
@@ -30,7 +29,7 @@ import ( | |
type ExMessage json.RawMessage | ||
|
||
func (m ExMessage) Structured(b binding.StructuredEncoder) error { | ||
return b.SetStructuredEvent(format.JSON, bytes.NewReader([]byte(m))) | ||
return b.SetStructuredEvent(format.JSON, &m) | ||
} | ||
|
||
func (m ExMessage) Binary(binding.BinaryEncoder) error { | ||
|
@@ -46,9 +45,22 @@ func (m ExMessage) Event(b binding.EventEncoder) error { | |
return b.SetEvent(e) | ||
} | ||
|
||
func (m *ExMessage) IsEmpty() bool { | ||
return m == nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you get rid of the pointer (m ExMessage) then this is (len(m) == 0) if you keep the pointer then it should be (m == nil || len(*m) == 0) |
||
} | ||
|
||
func (m *ExMessage) Bytes() []byte { | ||
return *m | ||
} | ||
|
||
func (m *ExMessage) Reader() io.Reader { | ||
return bytes.NewReader(m.Bytes()) | ||
} | ||
|
||
func (m ExMessage) Finish(error) error { return nil } | ||
|
||
var _ binding.Message = (*ExMessage)(nil) | ||
var _ binding.MessagePayloadReader = (*ExMessage)(nil) | ||
|
||
// ExSender sends by writing JSON encoded events to an io.Writer | ||
// ExSender supports transcoding | ||
|
@@ -63,6 +75,9 @@ func NewExSender(w io.Writer, factories ...binding.TransformerFactory) binding.S | |
} | ||
|
||
func (s *ExSender) Send(ctx context.Context, m binding.Message) error { | ||
// Invoke m.Finish to notify the receiver that message was processed | ||
defer func() { _ = m.Finish(nil) }() | ||
|
||
// Translate tries the various encodings, starting with provided root encoder factories. | ||
// If a sender doesn't support a specific encoding, a null root encoder factory could be provided. | ||
_, _, err := binding.Translate( | ||
|
@@ -79,13 +94,9 @@ func (s *ExSender) Send(ctx context.Context, m binding.Message) error { | |
return err | ||
} | ||
|
||
func (s *ExSender) SetStructuredEvent(f format.Format, event io.Reader) error { | ||
func (s *ExSender) SetStructuredEvent(f format.Format, event binding.MessagePayloadReader) error { | ||
if f == format.JSON { | ||
b, err := ioutil.ReadAll(event) | ||
if err != nil { | ||
return err | ||
} | ||
return s.encoder.Encode(json.RawMessage(b)) | ||
return s.encoder.Encode(event.Bytes()) | ||
} else { | ||
return binding.ErrNotStructured | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,8 @@ import ( | |
// to a Sender, they may not be implemented by all Message instances. A Sender should | ||
// try each method of interest and fall back to Event(EventEncoder) if none are supported. | ||
// | ||
// The message should be "consumable" several times, meaning that its methods to visit it can be called several times. | ||
// | ||
type Message interface { | ||
// Structured transfers a structured-mode event to a StructuredEncoder. | ||
// Returns ErrNotStructured if message is not in structured mode. | ||
|
@@ -84,13 +86,26 @@ var ErrNotStructured = errors.New("message is not in structured mode") | |
// ErrNotBinary returned by Message.Binary for non-binary messages. | ||
var ErrNotBinary = errors.New("message is not in binary mode") | ||
|
||
// MessagePayload allows to read a message payload or as a byte array or as a reader | ||
// Message implementers must re | ||
type MessagePayloadReader interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is an undue burden on the Message implementer. If they provide an io.Reader then io.Copy to a bytes.Buffer is maximally efficient, and the streaming case is supported. io.Reader is a standard Go idiom (net.http and many other standard libs) so I don't think we should hide it in a non-standard interface. |
||
// Returns true if the message payload is empty | ||
IsEmpty() bool | ||
|
||
// Returns the payload as a byte array, nil if IsEmpty() == true | ||
Bytes() []byte | ||
|
||
// Returns the payload as an io.Reader, nil if IsEmpty() == true | ||
Reader() io.Reader | ||
} | ||
|
||
// StructuredEncoder should generate a new representation of the event starting from a structured message. | ||
// | ||
// Protocols that supports structured encoding should implement this interface to implement direct | ||
// structured -> structured transfer. | ||
type StructuredEncoder interface { | ||
// Event receives an io.Reader for the whole event. | ||
SetStructuredEvent(format format.Format, event io.Reader) error | ||
SetStructuredEvent(format format.Format, event MessagePayloadReader) error | ||
} | ||
|
||
// BinaryEncoder should generate a new representation of the event starting from a binary message. | ||
|
@@ -100,7 +115,7 @@ type StructuredEncoder interface { | |
type BinaryEncoder interface { | ||
// SetData receives an io.Reader for the data attribute. | ||
// io.Reader could be empty, meaning that message payload is empty | ||
SetData(data io.Reader) error | ||
SetData(data MessagePayloadReader) error | ||
|
||
// Set a standard attribute. | ||
// | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to use *m is not necessary. json.RawMessage is just a type cast for []byte, and in Go slices are semantically pointers (actually pointer pairs) - passing or assigning a slice does not copy the underlying data. You need to use the copy() or append() built-in functions to copy the data. So passing/assigning ExMessage by value doesn't copy any data.