From baccf3fa098481586dd4c7c9d946294944917d1b Mon Sep 17 00:00:00 2001 From: Enrique Lacal Date: Thu, 29 Jun 2023 13:58:49 +0100 Subject: [PATCH 01/10] feat: batching events and webhook plugin support Signed-off-by: Enrique Lacal --- docs/reference/types/subscription.md | 4 +- docs/reference/types/wsstart.md | 4 +- docs/swagger/swagger.yaml | 120 ++++++++++++ internal/coremsgs/en_struct_descriptions.go | 10 +- internal/events/event_dispatcher.go | 98 +++++++++- internal/events/event_dispatcher_test.go | 52 ++++++ internal/events/system/events.go | 4 + internal/events/webhooks/webhooks.go | 193 ++++++++++++++++++++ internal/events/websockets/websockets.go | 4 + internal/orchestrator/subscriptions.go | 8 + mocks/eventsmocks/plugin.go | 14 ++ pkg/core/subscription.go | 8 +- pkg/events/plugin.go | 4 + 13 files changed, 507 insertions(+), 16 deletions(-) diff --git a/docs/reference/types/subscription.md b/docs/reference/types/subscription.md index c09ce3ae36..641de927cc 100644 --- a/docs/reference/types/subscription.md +++ b/docs/reference/types/subscription.md @@ -105,6 +105,8 @@ nav_order: 3 | `firstEvent` | Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest' | `SubOptsFirstEvent` | | `readAhead` | The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts | `uint16` | | `withData` | Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports. | `bool` | +| `batch` | Enable batching. Works in conjunction with readAhead which defines the batchSize. | `bool` | +| `batchTimeout` | When batching is enabled, the optional timeout to send events even when the batch hasn't filled. | `string` | | `fastack` | Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations | `bool` | | `url` | Webhooks only: HTTP url to invoke. Can be relative if a base URL is set in the webhook plugin config | `string` | | `method` | Webhooks only: HTTP method to invoke. Default=POST | `string` | @@ -148,7 +150,7 @@ nav_order: 3 | `requestTimeout` | The max duration to hold a TLS handshake alive | `string` | | `maxIdleConns` | The max number of idle connections to hold pooled | `int` | | `idleTimeout` | The max duration to hold a HTTP keepalive connection between calls | `string` | -| `connectionTimeout` | | `string` | +| `connectionTimeout` | The maximum amount of time that a connection is allowed to remain with no data transmitted. | `string` | | `expectContinueTimeout` | See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) | `string` | diff --git a/docs/reference/types/wsstart.md b/docs/reference/types/wsstart.md index 880718c9a7..6690a07a4b 100644 --- a/docs/reference/types/wsstart.md +++ b/docs/reference/types/wsstart.md @@ -96,6 +96,8 @@ nav_order: 23 | `firstEvent` | Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest' | `SubOptsFirstEvent` | | `readAhead` | The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts | `uint16` | | `withData` | Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports. | `bool` | +| `batch` | Enable batching. Works in conjunction with readAhead which defines the batchSize. | `bool` | +| `batchTimeout` | When batching is enabled, the optional timeout to send events even when the batch hasn't filled. | `string` | | `fastack` | Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations | `bool` | | `url` | Webhooks only: HTTP url to invoke. Can be relative if a base URL is set in the webhook plugin config | `string` | | `method` | Webhooks only: HTTP method to invoke. Default=POST | `string` | @@ -139,7 +141,7 @@ nav_order: 23 | `requestTimeout` | The max duration to hold a TLS handshake alive | `string` | | `maxIdleConns` | The max number of idle connections to hold pooled | `int` | | `idleTimeout` | The max duration to hold a HTTP keepalive connection between calls | `string` | -| `connectionTimeout` | | `string` | +| `connectionTimeout` | The maximum amount of time that a connection is allowed to remain with no data transmitted. | `string` | | `expectContinueTimeout` | See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) | `string` | diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml index a3381ed6f7..2053a2bcae 100644 --- a/docs/swagger/swagger.yaml +++ b/docs/swagger/swagger.yaml @@ -27701,6 +27701,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with + readAhead which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel @@ -27724,6 +27732,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -27973,6 +27983,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -27995,6 +28013,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -28226,6 +28246,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -28248,6 +28276,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -28494,6 +28524,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -28516,6 +28554,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -28747,6 +28787,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -28769,6 +28817,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -29077,6 +29127,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -29099,6 +29157,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -36544,6 +36604,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with + readAhead which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel @@ -36567,6 +36635,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -36809,6 +36879,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -36831,6 +36909,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -37062,6 +37142,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -37084,6 +37172,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -37323,6 +37413,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -37345,6 +37443,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -37576,6 +37676,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -37598,6 +37706,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) @@ -37892,6 +38002,14 @@ paths: options: description: Subscription options properties: + batch: + description: Enable batching. Works in conjunction with readAhead + which defines the batchSize. + type: boolean + batchTimeout: + description: When batching is enabled, the optional timeout + to send events even when the batch hasn't filled. + type: string fastack: description: 'Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations' @@ -37914,6 +38032,8 @@ paths: description: 'Webhooks only: a set of options for HTTP' properties: connectionTimeout: + description: The maximum amount of time that a connection + is allowed to remain with no data transmitted. type: string expectContinueTimeout: description: See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport) diff --git a/internal/coremsgs/en_struct_descriptions.go b/internal/coremsgs/en_struct_descriptions.go index 24e792e1b2..199fab5e37 100644 --- a/internal/coremsgs/en_struct_descriptions.go +++ b/internal/coremsgs/en_struct_descriptions.go @@ -537,9 +537,11 @@ var ( SubscriptionBlockchainEventFilterListener = ffm("SubscriptionBlockchainEventFilter.listener", "Regular expression to apply to the blockchain event 'listener' field, which is the UUID of the event listener. So you can restrict your subscription to certain blockchain listeners. Alternatively to avoid your application need to know listener UUIDs you can set the 'topic' field of blockchain event listeners, and use a topic filter on your subscriptions") // SubscriptionCoreOptions field descriptions - SubscriptionCoreOptionsFirstEvent = ffm("SubscriptionCoreOptions.firstEvent", "Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest'") - SubscriptionCoreOptionsReadAhead = ffm("SubscriptionCoreOptions.readAhead", "The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts") - SubscriptionCoreOptionsWithData = ffm("SubscriptionCoreOptions.withData", "Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports.") + SubscriptionCoreOptionsFirstEvent = ffm("SubscriptionCoreOptions.firstEvent", "Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest'") + SubscriptionCoreOptionsReadAhead = ffm("SubscriptionCoreOptions.readAhead", "The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts") + SubscriptionCoreOptionsWithData = ffm("SubscriptionCoreOptions.withData", "Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports.") + SubscriptionCoreOptionsBatch = ffm("SubscriptionCoreOptions.batch", "Enable batching. Works in conjunction with readAhead which defines the batchSize.") + SubscriptionCoreOptionsBatchTimeout = ffm("SubscriptionCoreOptions.batchTimeout", "When batching is enabled, the optional timeout to send events even when the batch hasn't filled.") // TokenApproval field descriptions TokenApprovalLocalID = ffm("TokenApproval.localId", "The UUID of this token approval, in the local FireFly node") @@ -706,7 +708,7 @@ var ( WebhookOptHTTPExpectContinueTimeout = ffm("WebhookHTTPOptions.expectContinueTimeout", "See [ExpectContinueTimeout in the Go docs](https://pkg.go.dev/net/http#Transport)") WebhookOptHTTPIdleTimeout = ffm("WebhookHTTPOptions.idleTimeout", "The max duration to hold a HTTP keepalive connection between calls") WebhookOptHTTPMaxIdleConns = ffm("WebhookHTTPOptions.maxIdleConns", "The max number of idle connections to hold pooled") - WebhookOptHTTPConnectionTimeout = ffm("WebhookHTTPOptions.connectionTimeout", "") + WebhookOptHTTPConnectionTimeout = ffm("WebhookHTTPOptions.connectionTimeout", "The maximum amount of time that a connection is allowed to remain with no data transmitted.") WebhookOptHTTPTLSHandshakeTimeout = ffm("WebhookHTTPOptions.tlsHandshakeTimeout", "The max duration to hold a TLS handshake alive") WebhookOptHTTPRequestTimeout = ffm("WebhookHTTPOptions.requestTimeout", "The max duration to hold a TLS handshake alive") diff --git a/internal/events/event_dispatcher.go b/internal/events/event_dispatcher.go index 94e0b477fa..0b0a0642cb 100644 --- a/internal/events/event_dispatcher.go +++ b/internal/events/event_dispatcher.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sync" + "time" "github.com/hyperledger/firefly-common/pkg/config" "github.com/hyperledger/firefly-common/pkg/ffapi" @@ -39,7 +40,8 @@ import ( ) const ( - maxReadAhead = 65536 + maxReadAhead = 65536 + defaultBatchTimeout = time.Duration(2) * time.Second ) type ackNack struct { @@ -67,6 +69,8 @@ type eventDispatcher struct { mux sync.Mutex namespace string readAhead int + batch bool + batchTimeout time.Duration subscription *subscription txHelper txcommon.Helper } @@ -80,6 +84,11 @@ func newEventDispatcher(ctx context.Context, enricher *eventEnricher, ei events. if readAhead > maxReadAhead { readAhead = maxReadAhead } + + batchTimeout := defaultBatchTimeout + if sub.definition.Options.BatchTimeout != "" { + batchTimeout = fftypes.ParseToDuration(sub.definition.Options.BatchTimeout) + } ed := &eventDispatcher{ ctx: log.WithLogField(log.WithLogField(ctx, "role", fmt.Sprintf("ed[%s]", connID)), @@ -100,6 +109,8 @@ func newEventDispatcher(ctx context.Context, enricher *eventEnricher, ei events. acksNacks: make(chan ackNack), closed: make(chan struct{}), txHelper: txHelper, + batch: sub.definition.Options.Batch, + batchTimeout: batchTimeout, } pollerConf := &eventPollerConf{ @@ -149,7 +160,12 @@ func (ed *eventDispatcher) electAndStart() { // We're ready to go - not ed.elected = true ed.eventPoller.start() - go ed.deliverEvents() + + if ed.batch { + go ed.deliverBatchedEvents() + } else { + go ed.deliverEvents() + } // Wait until the event poller closes <-ed.eventPoller.closed } @@ -284,22 +300,22 @@ func (ed *eventDispatcher) bufferedDelivery(events []core.LocallySequenced) (boo // or a reset event happens for { ed.mux.Lock() - var disapatchable []*core.EventDelivery + var dispatchable []*core.EventDelivery inflightCount := len(ed.inflight) maxDispatch := 1 + ed.readAhead - inflightCount if maxDispatch >= len(matching) { - disapatchable = matching + dispatchable = matching matching = nil } else if maxDispatch > 0 { - disapatchable = matching[0:maxDispatch] + dispatchable = matching[0:maxDispatch] matching = matching[maxDispatch:] } ed.mux.Unlock() l.Debugf("Dispatcher event state: readahead=%d candidates=%d matched=%d inflight=%d queued=%d dispatched=%d dispatchable=%d lastAck=%d nacks=%d highest=%d", - ed.readAhead, len(candidates), matchCount, inflightCount, len(matching), dispatched, len(disapatchable), lastAck, nacks, highestOffset) + ed.readAhead, len(candidates), matchCount, inflightCount, len(matching), dispatched, len(dispatchable), lastAck, nacks, highestOffset) - for _, event := range disapatchable { + for _, event := range dispatchable { ed.mux.Lock() ed.inflight[*event.ID] = &event.Event inflightCount = len(ed.inflight) @@ -364,6 +380,72 @@ func (ed *eventDispatcher) handleAckOffsetUpdate(ack ackNack) { } } +func (ed *eventDispatcher) deliverBatchedEvents() { + withData := ed.subscription.definition.Options.WithData != nil && *ed.subscription.definition.Options.WithData + + var events []*core.EventDelivery + var dataSet []core.DataArray + var batchTimeoutContext context.Context + var batchTimeoutCancel func() + for { + var timeoutContext context.Context + var timedOut bool + if batchTimeoutContext != nil { + timeoutContext = batchTimeoutContext + } else { + timeoutContext = ed.ctx + } + select { + case event, ok := <-ed.eventDelivery: + if !ok { + if batchTimeoutCancel != nil { + batchTimeoutCancel() + } + return + } + + if events == nil && dataSet == nil { + events = []*core.EventDelivery{} + dataSet = []core.DataArray{} + batchTimeoutContext, batchTimeoutCancel = context.WithTimeout(ed.ctx, ed.batchTimeout) + } + + log.L(ed.ctx).Debugf("Dispatching %s event in a batch: %.10d/%s [%s]: ref=%s/%s", ed.transport.Name(), event.Sequence, event.ID, event.Type, event.Namespace, event.Reference) + + events = append(events, event) + + var data []*core.Data + var err error + if withData && event.Message != nil { + data, _, err = ed.data.GetMessageDataCached(ed.ctx, event.Message) + dataSet = append(dataSet, data) + } + + if err != nil { + ed.deliveryResponse(&core.EventDeliveryResponse{ID: event.ID, Rejected: true}) + } + + case <-timeoutContext.Done(): + timedOut = true + case <-ed.ctx.Done(): + if batchTimeoutCancel != nil { + batchTimeoutCancel() + } + return + } + + if len(events) >= ed.readAhead || (timedOut && len(events) > 0) { + // TODO properly handle the error + batchTimeoutCancel() + _ = ed.transport.DeliveryBatchRequest(ed.ctx, ed.connID, ed.subscription.definition, events, dataSet) + // If err handle all the delivery responses for all the events?? + events = nil + dataSet = nil + } + } +} + +// TODO issue here, we can't just call DeliveryRequest with one thing. func (ed *eventDispatcher) deliverEvents() { withData := ed.subscription.definition.Options.WithData != nil && *ed.subscription.definition.Options.WithData for { @@ -372,12 +454,14 @@ func (ed *eventDispatcher) deliverEvents() { if !ok { return } + log.L(ed.ctx).Debugf("Dispatching %s event: %.10d/%s [%s]: ref=%s/%s", ed.transport.Name(), event.Sequence, event.ID, event.Type, event.Namespace, event.Reference) var data []*core.Data var err error if withData && event.Message != nil { data, _, err = ed.data.GetMessageDataCached(ed.ctx, event.Message) } + if err == nil { err = ed.transport.DeliveryRequest(ed.ctx, ed.connID, ed.subscription.definition, event, data) } diff --git a/internal/events/event_dispatcher_test.go b/internal/events/event_dispatcher_test.go index 41ee0f35b7..812fcae416 100644 --- a/internal/events/event_dispatcher_test.go +++ b/internal/events/event_dispatcher_test.go @@ -1064,3 +1064,55 @@ func TestEventDispatcherWithReply(t *testing.T) { mbm.AssertExpectations(t) mms.AssertExpectations(t) } + +func TestEventDeliveryBatch(t *testing.T) { + log.SetLevel("debug") + var five = uint16(5) + sub := &subscription{ + dispatcherElection: make(chan bool, 1), + definition: &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ID: fftypes.NewUUID(), Namespace: "ns1", Name: "sub1"}, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + ReadAhead: &five, + Batch: true, + }, + }, + }, + eventMatcher: regexp.MustCompile(fmt.Sprintf("^%s|%s$", core.EventTypeMessageConfirmed, core.EventTypeMessageConfirmed)), + } + + ed, cancel := newTestEventDispatcher(sub) + cancel() + ed.acksNacks = make(chan ackNack, 5) + + event1 := fftypes.NewUUID() + ed.inflight[*event1] = &core.Event{ + ID: event1, + Namespace: "ns1", + } + + mms := &syncasyncmocks.Sender{} + mbm := ed.broadcast.(*broadcastmocks.Manager) + mbm.On("NewBroadcast", mock.Anything).Return(mms) + mms.On("Send", mock.Anything).Return(nil) + + ed.deliveryResponse(&core.EventDeliveryResponse{ + ID: event1, + Reply: &core.MessageInOut{ + Message: core.Message{ + Header: core.MessageHeader{ + Tag: "myreplytag1", + CID: fftypes.NewUUID(), + Type: core.MessageTypeBroadcast, + }, + }, + InlineData: core.InlineData{ + {Value: fftypes.JSONAnyPtr(`"my reply"`)}, + }, + }, + }) + + mbm.AssertExpectations(t) + mms.AssertExpectations(t) +} diff --git a/internal/events/system/events.go b/internal/events/system/events.go index 4cdd0bdc30..d50b8b57ca 100644 --- a/internal/events/system/events.go +++ b/internal/events/system/events.go @@ -134,6 +134,10 @@ func (se *Events) DeliveryRequest(ctx context.Context, connID string, sub *core. return nil } +func (se *Events) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { + return nil +} + func (se *Events) NamespaceRestarted(ns string, startTime time.Time) { // no-op } diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index cdd2e36e08..b67d6be83d 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -444,6 +444,155 @@ func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, s } } +func (wh *WebHooks) attemptBatchRequest(ctx context.Context, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) (req *whRequest, res *whResponse, err error) { + withData := sub.Options.WithData != nil && *sub.Options.WithData + + allData := make([]*fftypes.JSONAny, 0, len(data)) + + if withData { + // We can either append all the data as flat map + // or nest it based on the event name? + // TODO ask as part of review if we want to support this + for _, d := range data { + for _, element := range d { + if element.Value != nil { + allData = append(allData, element.Value) + } + } + } + } + + client := wh.client + if sub.Options.RestyClient != nil { + client = sub.Options.RestyClient + } + + req, err = wh.buildRequest(ctx, client, sub.Options.TransportOptions(), nil) + if err != nil { + return nil, nil, err + } + + // Need to play around with these + if req.method == http.MethodPost || req.method == http.MethodPatch || req.method == http.MethodPut { + switch { + case req.body != nil: + // We might have been told to extract a body from the first data record + req.r.SetBody(req.body) + case len(allData) > 0: + // We've got an array of data to POST + // Send all the data of each request + req.r.SetBody(allData) + default: + // Just send the event itself + req.r.SetBody(events) + } + } + + resp, err := req.r.Execute(req.method, req.url) + if err != nil { + return nil, nil, err + } + defer func() { _ = resp.RawBody().Close() }() + + res = &whResponse{ + Status: resp.StatusCode(), + Headers: fftypes.JSONObject{}, + } + + header := resp.Header() + for h := range header { + res.Headers[h] = header.Get(h) + } + contentType := header.Get("Content-Type") + log.L(ctx).Debugf("Response content-type '%s' forceJSON=%t", contentType, req.forceJSON) + if req.forceJSON { + contentType = "application/json" + } + res.Headers["Content-Type"] = contentType + if req.forceJSON || strings.HasPrefix(contentType, "application/json") { + var resData interface{} + err = json.NewDecoder(resp.RawBody()).Decode(&resData) + if err != nil { + return nil, nil, i18n.WrapError(ctx, err, coremsgs.MsgWebhooksReplyBadJSON) + } + b, _ := json.Marshal(&resData) // we know we can re-marshal It + res.Body = fftypes.JSONAnyPtrBytes(b) + } else { + // Anything other than JSON, gets returned as a JSON string in base64 encoding + buf := &bytes.Buffer{} + buf.WriteByte('"') + b64Encoder := base64.NewEncoder(base64.StdEncoding, buf) + _, _ = io.Copy(b64Encoder, resp.RawBody()) + _ = b64Encoder.Close() + buf.WriteByte('"') + res.Body = fftypes.JSONAnyPtrBytes(buf.Bytes()) + } + + return req, res, nil +} + +func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray, fastAck bool) { + req, res, gwErr := wh.attemptBatchRequest(ctx, sub, events, data) + if gwErr != nil { + // Generate a bad-gateway error response - we always want to send something back, + // rather than just causing timeouts + log.L(wh.ctx).Errorf("Failed to invoke webhook: %s", gwErr) + b, _ := json.Marshal(&fftypes.RESTError{ + Error: gwErr.Error(), + }) + res = &whResponse{ + Status: http.StatusBadGateway, + Headers: fftypes.JSONObject{ + "Content-Type": "application/json", + }, + Body: fftypes.JSONAnyPtrBytes(b), + } + } + b, _ := json.Marshal(&res) + log.L(wh.ctx).Tracef("Webhook response: %s", string(b)) + + // For each event emit a respons + for _, event := range events { + // Emit the response + if reply && event.Message != nil { + txType := fftypes.FFEnum(strings.ToLower(sub.Options.TransportOptions().GetString("replytx"))) + if req != nil && req.replyTx != "" { + txType = fftypes.FFEnum(strings.ToLower(req.replyTx)) + } + if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { + log.L(wh.ctx).Debugf("Sending reply message for %s CID=%s", event.ID, event.Message.Header.ID) + cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ + ID: event.ID, + Rejected: false, + Subscription: event.Subscription, + Reply: &core.MessageInOut{ + Message: core.Message{ + Header: core.MessageHeader{ + CID: event.Message.Header.ID, + Group: event.Message.Header.Group, + Type: event.Message.Header.Type, + Topics: event.Message.Header.Topics, + Tag: sub.Options.TransportOptions().GetString("replytag"), + TxType: txType, + }, + }, + InlineData: core.InlineData{ + {Value: fftypes.JSONAnyPtrBytes(b)}, + }, + }, + }) + } + } else if !fastAck { + if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { + cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ + ID: event.ID, + Rejected: false, + Subscription: event.Subscription, + }) + } + } + } +} func (wh *WebHooks) DeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event *core.EventDelivery, data core.DataArray) error { reply := sub.Options.TransportOptions().GetBool("reply") if reply && event.Message != nil && event.Message.Header.CID != nil { @@ -476,10 +625,54 @@ func (wh *WebHooks) DeliveryRequest(ctx context.Context, connID string, sub *cor return nil } + // NOTE: We could check here for batching and accumulate but we can't return because this causes the offset to jump... + + // TODO we don't look at the error here? wh.doDelivery(ctx, connID, reply, sub, event, data, false) return nil } +func (wh *WebHooks) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { + reply := sub.Options.TransportOptions().GetBool("reply") + // if reply && event.Message != nil && event.Message.Header.CID != nil { + // // We cowardly refuse to dispatch a message that is itself a reply, as it's hard for users to + // // avoid loops - and there's no way for us to detect here if a user has configured correctly + // // to avoid a loop. + // log.L(wh.ctx).Debugf("Webhook subscription with reply enabled called with reply event '%s'", event.ID) + // if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { + // cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ + // ID: event.ID, + // Rejected: false, + // Subscription: event.Subscription, + // }) + // } + // return nil + // } + + // // In fastack mode we drive calls in parallel to the backend, immediately acknowledging the event + // NOTE: We cannot use this with reply mode, as when we're sending a reply the `DeliveryResponse` + // callback must include the reply in-line. + if !reply && sub.Options.TransportOptions().GetBool("fastack") { + for _, event := range events { + if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { + cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ + ID: event.ID, + Rejected: false, + Subscription: event.Subscription, + }) + } + } + go wh.doBatchedDelivery(ctx, connID, reply, sub, events, data, true) + return nil + } + + // NOTE: We could check here for batching and accumulate but we can't return because this causes the offset to jump... + + // TODO we don't look at the error here? + wh.doBatchedDelivery(ctx, connID, reply, sub, events, data, false) + return nil +} + func (wh *WebHooks) NamespaceRestarted(ns string, startTime time.Time) { // no-op } diff --git a/internal/events/websockets/websockets.go b/internal/events/websockets/websockets.go index 29e431d17f..2c23197f80 100644 --- a/internal/events/websockets/websockets.go +++ b/internal/events/websockets/websockets.go @@ -217,3 +217,7 @@ func (ws *WebSockets) GetStatus() *core.WebSocketStatus { } return status } + +func (ws *WebSockets) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { + return nil +} diff --git a/internal/orchestrator/subscriptions.go b/internal/orchestrator/subscriptions.go index f782dddb92..8f1989b1a4 100644 --- a/internal/orchestrator/subscriptions.go +++ b/internal/orchestrator/subscriptions.go @@ -18,6 +18,7 @@ package orchestrator import ( "context" + "time" "github.com/hyperledger/firefly-common/pkg/ffapi" "github.com/hyperledger/firefly-common/pkg/fftypes" @@ -56,6 +57,13 @@ func (or *orchestrator) createUpdateSubscription(ctx context.Context, subDef *co subDef.Options.TLSConfig = or.namespace.TLSConfigs[subDef.Options.TLSConfigName] } + if subDef.Options.BatchTimeout != "" { + _, err := fftypes.ParseDurationString(subDef.Options.BatchTimeout, time.Millisecond) + if err != nil { + return nil, err + } + } + return subDef, or.events.CreateUpdateDurableSubscription(ctx, subDef, mustNew) } diff --git a/mocks/eventsmocks/plugin.go b/mocks/eventsmocks/plugin.go index f62e8fe33b..1ff2d33143 100644 --- a/mocks/eventsmocks/plugin.go +++ b/mocks/eventsmocks/plugin.go @@ -37,6 +37,20 @@ func (_m *Plugin) Capabilities() *events.Capabilities { return r0 } +// DeliveryBatchRequest provides a mock function with given fields: ctx, connID, sub, event, data +func (_m *Plugin) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error { + ret := _m.Called(ctx, connID, sub, event, data) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, *core.Subscription, []*core.EventDelivery, []core.DataArray) error); ok { + r0 = rf(ctx, connID, sub, event, data) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // DeliveryRequest provides a mock function with given fields: ctx, connID, sub, event, data func (_m *Plugin) DeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event *core.EventDelivery, data core.DataArray) error { ret := _m.Called(ctx, connID, sub, event, data) diff --git a/pkg/core/subscription.go b/pkg/core/subscription.go index 48851fdd32..20135c8e24 100644 --- a/pkg/core/subscription.go +++ b/pkg/core/subscription.go @@ -89,9 +89,11 @@ const ( // SubscriptionCoreOptions are the core options that apply across all transports type SubscriptionCoreOptions struct { - FirstEvent *SubOptsFirstEvent `ffstruct:"SubscriptionCoreOptions" json:"firstEvent,omitempty"` - ReadAhead *uint16 `ffstruct:"SubscriptionCoreOptions" json:"readAhead,omitempty"` - WithData *bool `ffstruct:"SubscriptionCoreOptions" json:"withData,omitempty"` + FirstEvent *SubOptsFirstEvent `ffstruct:"SubscriptionCoreOptions" json:"firstEvent,omitempty"` + ReadAhead *uint16 `ffstruct:"SubscriptionCoreOptions" json:"readAhead,omitempty"` + WithData *bool `ffstruct:"SubscriptionCoreOptions" json:"withData,omitempty"` + Batch bool `ffstruct:"SubscriptionCoreOptions" json:"batch,omitempty"` + BatchTimeout string `ffstruct:"SubscriptionCoreOptions" json:"batchTimeout,omitempty"` } // SubscriptionOptions customize the behavior of subscriptions diff --git a/pkg/events/plugin.go b/pkg/events/plugin.go index a357dad03c..d1d51126e2 100644 --- a/pkg/events/plugin.go +++ b/pkg/events/plugin.go @@ -51,6 +51,10 @@ type Plugin interface { // Data will only be supplied as non-nil if the subscription is set to include data DeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event *core.EventDelivery, data core.DataArray) error + // DeliveryBatchRequest requests delivery of multiple events on a connection, which must later be responded to + // Data will only be supplied as non-nil if the subscription is set to include data + DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error + // NamespaceRestarted is called after a namespace restarts. For a connect-in style plugin, like // WebSockets, this must re-register any active connections that started before the time passed in. NamespaceRestarted(ns string, startTime time.Time) From 07a12d98b56b7652862c9a3b640c18c572920939 Mon Sep 17 00:00:00 2001 From: Enrique Lacal Date: Thu, 6 Jul 2023 16:37:54 +0100 Subject: [PATCH 02/10] test: testing batching part 1 Signed-off-by: Enrique Lacal --- internal/events/event_dispatcher.go | 6 +- internal/events/event_dispatcher_test.go | 180 ++++++++++++++++++ internal/events/system/events.go | 2 +- internal/events/system/events_test.go | 13 ++ internal/events/webhooks/webhooks.go | 57 ++++-- internal/events/webhooks/webhooks_test.go | 140 ++++++++++++++ internal/events/websockets/websockets.go | 2 +- internal/events/websockets/websockets_test.go | 14 ++ mocks/eventsmocks/plugin.go | 28 +-- pkg/events/plugin.go | 2 +- 10 files changed, 404 insertions(+), 40 deletions(-) diff --git a/internal/events/event_dispatcher.go b/internal/events/event_dispatcher.go index 0b0a0642cb..51f6a0f4e1 100644 --- a/internal/events/event_dispatcher.go +++ b/internal/events/event_dispatcher.go @@ -434,10 +434,8 @@ func (ed *eventDispatcher) deliverBatchedEvents() { return } - if len(events) >= ed.readAhead || (timedOut && len(events) > 0) { - // TODO properly handle the error - batchTimeoutCancel() - _ = ed.transport.DeliveryBatchRequest(ed.ctx, ed.connID, ed.subscription.definition, events, dataSet) + if len(events) == ed.readAhead || (timedOut && len(events) > 0) { + _ = ed.transport.BatchDeliveryRequest(ed.ctx, ed.connID, ed.subscription.definition, events, dataSet) // If err handle all the delivery responses for all the events?? events = nil dataSet = nil diff --git a/internal/events/event_dispatcher_test.go b/internal/events/event_dispatcher_test.go index 812fcae416..7060d66118 100644 --- a/internal/events/event_dispatcher_test.go +++ b/internal/events/event_dispatcher_test.go @@ -932,6 +932,37 @@ func TestEventDeliveryClosed(t *testing.T) { cancel() } +// func TestBatchEventDeliveryClosed(t *testing.T) { + +// sub := &subscription{ +// definition: &core.Subscription{}, +// } +// ed, cancel := newTestEventDispatcher(sub) + +// go ed.deliverBatchedEvents() + +// id1 := fftypes.NewUUID() +// ed.eventDelivery <- &core.EventDelivery{ +// EnrichedEvent: core.EnrichedEvent{ +// Event: core.Event{ +// ID: id1, +// }, +// Message: &core.Message{ +// Header: core.MessageHeader{ +// ID: fftypes.NewUUID(), +// }, +// Data: core.DataRefs{ +// {ID: fftypes.NewUUID()}, +// }, +// }, +// }, +// } +// time.Sleep(1000) +// close(ed.eventDelivery) +// time.Sleep(1000) +// cancel() +// } + func TestAckClosed(t *testing.T) { sub := &subscription{ @@ -1116,3 +1147,152 @@ func TestEventDeliveryBatch(t *testing.T) { mbm.AssertExpectations(t) mms.AssertExpectations(t) } + +func TestEventDispatcherBatchReadAhead(t *testing.T) { + log.SetLevel("debug") + var five = uint16(5) + subID := fftypes.NewUUID() + sub := &subscription{ + dispatcherElection: make(chan bool, 1), + definition: &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ID: subID, Namespace: "ns1", Name: "sub1"}, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + ReadAhead: &five, + Batch: true, + BatchTimeout: "1s", + }, + }, + }, + eventMatcher: regexp.MustCompile(fmt.Sprintf("^%s|%s$", core.EventTypeMessageConfirmed, core.EventTypeMessageConfirmed)), + } + + ed, cancel := newTestEventDispatcher(sub) + defer cancel() + go ed.deliverBatchedEvents() + ed.eventPoller.offsetCommitted = make(chan int64, 3) + mdi := ed.database.(*databasemocks.Plugin) + mei := ed.transport.(*eventsmocks.Plugin) + mdm := ed.data.(*datamocks.Manager) + + eventDeliveries := make(chan *core.EventDelivery) + deliveryRequestMock := mei.On("BatchDeliveryRequest", ed.ctx, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + deliveryRequestMock.RunFn = func(a mock.Arguments) { + batchEvents := a.Get(3).([]*core.EventDelivery) + for _, event := range batchEvents { + eventDeliveries <- event + } + } + + // Setup the IDs + ref1 := fftypes.NewUUID() + ev1 := fftypes.NewUUID() + ref2 := fftypes.NewUUID() + ev2 := fftypes.NewUUID() + ref3 := fftypes.NewUUID() + ev3 := fftypes.NewUUID() + ref4 := fftypes.NewUUID() + ev4 := fftypes.NewUUID() + + // Setup enrichment + mdm.On("GetMessageWithDataCached", mock.Anything, ref1).Return(&core.Message{ + Header: core.MessageHeader{ID: ref1}, + }, nil, true, nil) + mdm.On("GetMessageWithDataCached", mock.Anything, ref2).Return(&core.Message{ + Header: core.MessageHeader{ID: ref2}, + }, nil, true, nil) + mdm.On("GetMessageWithDataCached", mock.Anything, ref3).Return(&core.Message{ + Header: core.MessageHeader{ID: ref3}, + }, nil, true, nil) + mdm.On("GetMessageWithDataCached", mock.Anything, ref4).Return(&core.Message{ + Header: core.MessageHeader{ID: ref4}, + }, nil, true, nil) + + // Deliver a batch of messages + batch1Done := make(chan struct{}) + go func() { + repoll, err := ed.bufferedDelivery([]core.LocallySequenced{ + &core.Event{ID: ev1, Sequence: 10000001, Reference: ref1, Type: core.EventTypeMessageConfirmed}, // match + &core.Event{ID: ev2, Sequence: 10000002, Reference: ref2, Type: core.EventTypeMessageRejected}, + &core.Event{ID: ev3, Sequence: 10000003, Reference: ref3, Type: core.EventTypeMessageConfirmed}, // match + &core.Event{ID: ev4, Sequence: 10000004, Reference: ref4, Type: core.EventTypeMessageConfirmed}, // match + }) + assert.NoError(t, err) + assert.True(t, repoll) + close(batch1Done) + }() + + // Wait for the two calls to deliver the matching messages to the client (read ahead allows this) + event1 := <-eventDeliveries + assert.Equal(t, *ev1, *event1.ID) + assert.Equal(t, *ref1, *event1.Message.Header.ID) + event3 := <-eventDeliveries + assert.Equal(t, *ev3, *event3.ID) + assert.Equal(t, *ref3, *event3.Message.Header.ID) + event4 := <-eventDeliveries + assert.Equal(t, *ev4, *event4.ID) + assert.Equal(t, *ref4, *event4.Message.Header.ID) + + // Send back the two acks - out of order to validate the read-ahead logic + go func() { + ed.deliveryResponse(&core.EventDeliveryResponse{ID: event4.ID}) + ed.deliveryResponse(&core.EventDeliveryResponse{ID: event1.ID}) + ed.deliveryResponse(&core.EventDeliveryResponse{ID: event3.ID}) + }() + + // Confirm we get the offset updates in the correct order, even though the confirmations + // came in a different order from the app. + assert.Equal(t, int64(10000001), <-ed.eventPoller.offsetCommitted) + assert.Equal(t, int64(10000003), <-ed.eventPoller.offsetCommitted) + assert.Equal(t, int64(10000004), <-ed.eventPoller.offsetCommitted) + + // This should complete the batch + <-batch1Done + + mdi.AssertExpectations(t) + mei.AssertExpectations(t) + mdm.AssertExpectations(t) +} + +func TestBatchDeliverEventsWithDataFail(t *testing.T) { + yes := true + sub := &subscription{ + definition: &core.Subscription{ + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + WithData: &yes, + }, + }, + }, + } + + ed, cancel := newTestEventDispatcher(sub) + defer cancel() + + mdm := ed.data.(*datamocks.Manager) + mdm.On("GetMessageDataCached", ed.ctx, mock.Anything).Return(nil, false, fmt.Errorf("pop")) + + id1 := fftypes.NewUUID() + ed.eventDelivery <- &core.EventDelivery{ + EnrichedEvent: core.EnrichedEvent{ + Event: core.Event{ + ID: id1, + }, + Message: &core.Message{ + Header: core.MessageHeader{ + ID: fftypes.NewUUID(), + }, + Data: core.DataRefs{ + {ID: fftypes.NewUUID()}, + }, + }, + }, + } + + ed.inflight[*id1] = &core.Event{ID: id1} + go ed.deliverBatchedEvents() + + an := <-ed.acksNacks + assert.True(t, an.isNack) + +} diff --git a/internal/events/system/events.go b/internal/events/system/events.go index d50b8b57ca..6dbecb3060 100644 --- a/internal/events/system/events.go +++ b/internal/events/system/events.go @@ -134,7 +134,7 @@ func (se *Events) DeliveryRequest(ctx context.Context, connID string, sub *core. return nil } -func (se *Events) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { +func (se *Events) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { return nil } diff --git a/internal/events/system/events_test.go b/internal/events/system/events_test.go index d099c24fef..d85727bec3 100644 --- a/internal/events/system/events_test.go +++ b/internal/events/system/events_test.go @@ -142,3 +142,16 @@ func TestNamespaceRestarted(t *testing.T) { se.NamespaceRestarted("ns1", time.Now()) } + +func TestEventDeliveryBatch(t *testing.T) { + se, cancel := newTestEvents(t) + defer cancel() + + sub := &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Namespace: "ns1", + }, + } + + se.BatchDeliveryRequest(se.ctx, "id", sub, []*core.EventDelivery{}, []core.DataArray{}) +} diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index b67d6be83d..7205c1d1d4 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -460,6 +460,19 @@ func (wh *WebHooks) attemptBatchRequest(ctx context.Context, sub *core.Subscript } } } + + // if len(allData) == 0 { + // firstData = fftypes.JSONObject{} + // } else { + // // Use JSONObjectOk instead of JSONObject + // // JSONObject fails for datatypes such as array, string, bool, number etc + // firstData, valid = allData[0].JSONObjectOk() + // if !valid { + // firstData = fftypes.JSONObject{ + // "value": allData[0], + // } + // } + // } } client := wh.client @@ -551,7 +564,7 @@ func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply b, _ := json.Marshal(&res) log.L(wh.ctx).Tracef("Webhook response: %s", string(b)) - // For each event emit a respons + // For each event emit a response for _, event := range events { // Emit the response if reply && event.Message != nil { @@ -632,22 +645,31 @@ func (wh *WebHooks) DeliveryRequest(ctx context.Context, connID string, sub *cor return nil } -func (wh *WebHooks) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { +func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { reply := sub.Options.TransportOptions().GetBool("reply") - // if reply && event.Message != nil && event.Message.Header.CID != nil { - // // We cowardly refuse to dispatch a message that is itself a reply, as it's hard for users to - // // avoid loops - and there's no way for us to detect here if a user has configured correctly - // // to avoid a loop. - // log.L(wh.ctx).Debugf("Webhook subscription with reply enabled called with reply event '%s'", event.ID) - // if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { - // cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ - // ID: event.ID, - // Rejected: false, - // Subscription: event.Subscription, - // }) - // } - // return nil - // } + if reply { + nonReplyEvents := []*core.EventDelivery{} + for _, event := range events { + // We cowardly refuse to dispatch a message that is itself a reply, as it's hard for users to + // avoid loops - and there's no way for us to detect here if a user has configured correctly + // to avoid a loop. + if event.Message != nil && event.Message.Header.CID != nil { + log.L(wh.ctx).Debugf("Webhook subscription with reply enabled called with reply event '%s'", event.ID) + if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { + cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ + ID: event.ID, + Rejected: false, + Subscription: event.Subscription, + }) + } + continue + } + + nonReplyEvents = append(nonReplyEvents, event) + } + // Override the events to send without the reply events + events = nonReplyEvents + } // // In fastack mode we drive calls in parallel to the backend, immediately acknowledging the event // NOTE: We cannot use this with reply mode, as when we're sending a reply the `DeliveryResponse` @@ -666,9 +688,6 @@ func (wh *WebHooks) DeliveryBatchRequest(ctx context.Context, connID string, sub return nil } - // NOTE: We could check here for batching and accumulate but we can't return because this causes the offset to jump... - - // TODO we don't look at the error here? wh.doBatchedDelivery(ctx, connID, reply, sub, events, data, false) return nil } diff --git a/internal/events/webhooks/webhooks_test.go b/internal/events/webhooks/webhooks_test.go index 8e8398bfc2..c303b583bf 100644 --- a/internal/events/webhooks/webhooks_test.go +++ b/internal/events/webhooks/webhooks_test.go @@ -1216,3 +1216,143 @@ func TestNamespaceRestarted(t *testing.T) { wh.NamespaceRestarted("ns1", time.Now()) } + +func TestRequestWithBodyReplyEndToEndWithBatch(t *testing.T) { + wh, cancel := newTestWebHooks(t) + defer cancel() + + r := mux.NewRouter() + r.HandleFunc("/myapi", func(res http.ResponseWriter, req *http.Request) { + assert.Equal(t, "myheaderval", req.Header.Get("My-Header")) + assert.Equal(t, "myqueryval", req.URL.Query().Get("my-query")) + var data []fftypes.JSONObject + err := json.NewDecoder(req.Body).Decode(&data) + assert.NoError(t, err) + assert.Equal(t, len(data), 2) + assert.Equal(t, "inputvalue", data[0].GetObject("in_body").GetString("inputfield")) + res.Header().Set("my-reply-header", "myheaderval2") + res.WriteHeader(200) + res.Write([]byte(`{ + "replyfield": "replyvalue" + }`)) + }).Methods(http.MethodPut) + server := httptest.NewServer(r) + defer server.Close() + + yes := true + dataID := fftypes.NewUUID() + msgID := fftypes.NewUUID() + groupHash := fftypes.NewRandB32() + sub := &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Namespace: "ns1", + }, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + WithData: &yes, + }, + }, + } + to := sub.Options.TransportOptions() + to["reply"] = true + to["json"] = true + to["method"] = "PUT" + to["url"] = fmt.Sprintf("http://%s/myapi", server.Listener.Addr()) + to["headers"] = map[string]interface{}{ + "my-header": "myheaderval", + } + to["query"] = map[string]interface{}{ + "my-query": "myqueryval", + } + event1 := &core.EventDelivery{ + EnrichedEvent: core.EnrichedEvent{ + Event: core.Event{ + ID: fftypes.NewUUID(), + }, + Message: &core.Message{ + Header: core.MessageHeader{ + ID: msgID, + Group: groupHash, + Type: core.MessageTypePrivate, + }, + Data: core.DataRefs{ + {ID: dataID}, + }, + }, + }, + Subscription: core.SubscriptionRef{ + ID: sub.ID, + }, + } + + event2 := &core.EventDelivery{ + EnrichedEvent: core.EnrichedEvent{ + Event: core.Event{ + ID: fftypes.NewUUID(), + }, + Message: &core.Message{ + Header: core.MessageHeader{ + ID: msgID, + Group: groupHash, + Type: core.MessageTypePrivate, + }, + Data: core.DataRefs{ + {ID: dataID}, + }, + }, + }, + Subscription: core.SubscriptionRef{ + ID: sub.ID, + }, + } + + data1 := core.DataArray{&core.Data{ + ID: dataID, + Value: fftypes.JSONAnyPtr(`{ + "in_body": { + "inputfield": "inputvalue" + }, + "in_query": { + "dynamic-query": "dynamicqueryval" + }, + "in_headers": { + "dynamic-header": "dynamicheaderval" + }, + "in_path": "/my/sub/path?escape_query", + "in_replytx": true + }`), + }} + + data2 := core.DataArray{&core.Data{ + ID: dataID, + Value: fftypes.JSONAnyPtr(`{ + "in_body": { + "inputfield": "inputvalue" + }, + "in_query": { + "dynamic-query": "dynamicqueryval" + }, + "in_headers": { + "dynamic-header": "dynamicheaderval" + }, + "in_path": "/my/sub/path?escape_query", + "in_replytx": true + }`), + }} + + mcb := wh.callbacks.handlers["ns1"].(*eventsmocks.Callbacks) + mcb.On("DeliveryResponse", mock.Anything, mock.MatchedBy(func(response *core.EventDeliveryResponse) bool { + assert.Equal(t, *msgID, *response.Reply.Message.Header.CID) + assert.Equal(t, *groupHash, *response.Reply.Message.Header.Group) + assert.Equal(t, core.MessageTypePrivate, response.Reply.Message.Header.Type) + assert.Equal(t, "myheaderval2", response.Reply.InlineData[0].Value.JSONObject().GetObject("headers").GetString("My-Reply-Header")) + assert.Equal(t, "replyvalue", response.Reply.InlineData[0].Value.JSONObject().GetObject("body").GetString("replyfield")) + assert.Equal(t, float64(200), response.Reply.InlineData[0].Value.JSONObject()["status"]) + return true + })).Return(nil) + + err := wh.BatchDeliveryRequest(wh.ctx, mock.Anything, sub, []*core.EventDelivery{event1, event2}, []core.DataArray{data1, data2}) + assert.NoError(t, err) + + mcb.AssertExpectations(t) +} diff --git a/internal/events/websockets/websockets.go b/internal/events/websockets/websockets.go index 2c23197f80..e989164e17 100644 --- a/internal/events/websockets/websockets.go +++ b/internal/events/websockets/websockets.go @@ -218,6 +218,6 @@ func (ws *WebSockets) GetStatus() *core.WebSocketStatus { return status } -func (ws *WebSockets) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { +func (ws *WebSockets) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { return nil } diff --git a/internal/events/websockets/websockets_test.go b/internal/events/websockets/websockets_test.go index fef9ff5a75..471b849d4c 100644 --- a/internal/events/websockets/websockets_test.go +++ b/internal/events/websockets/websockets_test.go @@ -805,3 +805,17 @@ func TestNamespaceRestartedFailClose(t *testing.T) { mcb.AssertExpectations(t) } + +func TestEventDeliveryBatch(t *testing.T) { + cbs := &eventsmocks.Callbacks{} + ws, _, cancel := newTestWebsockets(t, cbs, nil) + defer cancel() + + sub := &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Namespace: "ns1", + }, + } + + ws.BatchDeliveryRequest(ws.ctx, "id", sub, []*core.EventDelivery{}, []core.DataArray{}) +} diff --git a/mocks/eventsmocks/plugin.go b/mocks/eventsmocks/plugin.go index 1ff2d33143..8ef7a2c124 100644 --- a/mocks/eventsmocks/plugin.go +++ b/mocks/eventsmocks/plugin.go @@ -21,6 +21,20 @@ type Plugin struct { mock.Mock } +// BatchDeliveryRequest provides a mock function with given fields: ctx, connID, sub, event, data +func (_m *Plugin) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error { + ret := _m.Called(ctx, connID, sub, event, data) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, *core.Subscription, []*core.EventDelivery, []core.DataArray) error); ok { + r0 = rf(ctx, connID, sub, event, data) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Capabilities provides a mock function with given fields: func (_m *Plugin) Capabilities() *events.Capabilities { ret := _m.Called() @@ -37,20 +51,6 @@ func (_m *Plugin) Capabilities() *events.Capabilities { return r0 } -// DeliveryBatchRequest provides a mock function with given fields: ctx, connID, sub, event, data -func (_m *Plugin) DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error { - ret := _m.Called(ctx, connID, sub, event, data) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, *core.Subscription, []*core.EventDelivery, []core.DataArray) error); ok { - r0 = rf(ctx, connID, sub, event, data) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // DeliveryRequest provides a mock function with given fields: ctx, connID, sub, event, data func (_m *Plugin) DeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event *core.EventDelivery, data core.DataArray) error { ret := _m.Called(ctx, connID, sub, event, data) diff --git a/pkg/events/plugin.go b/pkg/events/plugin.go index d1d51126e2..3b10be84ef 100644 --- a/pkg/events/plugin.go +++ b/pkg/events/plugin.go @@ -53,7 +53,7 @@ type Plugin interface { // DeliveryBatchRequest requests delivery of multiple events on a connection, which must later be responded to // Data will only be supplied as non-nil if the subscription is set to include data - DeliveryBatchRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error + BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error // NamespaceRestarted is called after a namespace restarts. For a connect-in style plugin, like // WebSockets, this must re-register any active connections that started before the time passed in. From 6160d87b32398bc8fee590c014507364bdd1bad7 Mon Sep 17 00:00:00 2001 From: Enrique Lacal Date: Thu, 6 Jul 2023 21:13:02 +0100 Subject: [PATCH 03/10] refactor into a create body function Signed-off-by: Enrique Lacal --- internal/events/webhooks/webhooks.go | 207 ++++++++-------------- internal/events/webhooks/webhooks_test.go | 5 +- 2 files changed, 79 insertions(+), 133 deletions(-) diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index 7205c1d1d4..1a93485917 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -286,31 +286,84 @@ func (wh *WebHooks) ValidateOptions(ctx context.Context, options *core.Subscript return err } -func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, event *core.EventDelivery, data core.DataArray) (req *whRequest, res *whResponse, err error) { - withData := sub.Options.WithData != nil && *sub.Options.WithData - allData := make([]*fftypes.JSONAny, 0, len(data)) - var firstData fftypes.JSONObject - var valid bool +func (wh *WebHooks) buildBody(events []*core.EventDelivery, data []core.DataArray, withData bool, batch bool) (body interface{}, firstData fftypes.JSONObject) { + if len(events) == 0 && len(data) == 0 { + return nil, nil + } + if withData { - for _, d := range data { - if d.Value != nil { - allData = append(allData, d.Value) + // We are in the case of batching + // [[{"event1data1": "stuff"}],[{"event2data1": "stuff"}]] + // [[{"event1data1": "stuff"},{"event1data2":"stuff"}],[{"event2data1": "stuff"},{"event2data2":"stuff"}]] + if len(data) > 1 && batch { + allData := [][]*fftypes.JSONAny{} + for _, eventData := range data { + allEventData := []*fftypes.JSONAny{} + for _, d := range eventData { + if d.Value != nil { + allEventData = append(allEventData, d.Value) + } + } + allData = append(allData, allEventData) } + return allData, nil } - if len(allData) == 0 { - firstData = fftypes.JSONObject{} - } else { - // Use JSONObjectOk instead of JSONObject - // JSONObject fails for datatypes such as array, string, bool, number etc - firstData, valid = allData[0].JSONObjectOk() - if !valid { - firstData = fftypes.JSONObject{ - "value": allData[0], + + if len(data) == 1 { + eventData := []*fftypes.JSONAny{} + for _, d := range data[0] { + if d.Value != nil { + eventData = append(eventData, d.Value) + } + } + + if len(eventData) == 0 { + // Send an empty object if ask withData but no data available + firstData = fftypes.JSONObject{} + body = firstData + } + + if len(eventData) >= 1 { + // Use JSONObjectOk instead of JSONObject + // JSONObject fails for datatypes such as array, string, bool, number etc + var valid bool + firstData, valid = eventData[0].JSONObjectOk() + if !valid { + firstData = fftypes.JSONObject{ + "value": eventData[0], + } + } + + if len(eventData) == 1 { + body = firstData + } else { + body = eventData } } + + return body, firstData } } + // We haven't returned yet so we are not in data + // We only send the events + if batch { + // [{"event1": "stuff"},"event2": "stuff"}] + // [{"event1": "stuff"}] + body = events + } else if len(events) == 1 { + // {"event1": "stuff"} + body = events[0] + } + + return body, firstData +} + +func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray, batch bool) (req *whRequest, res *whResponse, err error) { + withData := sub.Options.WithData != nil && *sub.Options.WithData + + body, firstData := wh.buildBody(events, data, withData, batch) + client := wh.client if sub.Options.RestyClient != nil { client = sub.Options.RestyClient @@ -326,23 +379,15 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, case req.body != nil: // We might have been told to extract a body from the first data record req.r.SetBody(req.body) - case len(allData) > 1: - // We've got an array of data to POST - req.r.SetBody(allData) - case len(allData) == 1: - // Just send the first object directly - req.r.SetBody(firstData) default: - // Just send the event itself - req.r.SetBody(event) - + req.r.SetBody(body) } } - log.L(wh.ctx).Debugf("Webhook-> %s %s event %s on subscription %s", req.method, req.url, event.ID, sub.ID) + // log.L(wh.ctx).Debugf("Webhook-> %s %s event %s on subscription %s", req.method, req.url, event.ID, sub.ID) resp, err := req.r.Execute(req.method, req.url) if err != nil { - log.L(ctx).Errorf("Webhook<- %s %s event %s on subscription %s failed: %s", req.method, req.url, event.ID, sub.ID, err) + // log.L(ctx).Errorf("Webhook<- %s %s event %s on subscription %s failed: %s", req.method, req.url, event.ID, sub.ID, err) return nil, nil, err } defer func() { _ = resp.RawBody().Close() }() @@ -351,7 +396,7 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, Status: resp.StatusCode(), Headers: fftypes.JSONObject{}, } - log.L(wh.ctx).Infof("Webhook<- %s %s event %s on subscription %s returned %d", req.method, req.url, event.ID, sub.ID, res.Status) + // log.L(wh.ctx).Infof("Webhook<- %s %s event %s on subscription %s returned %d", req.method, req.url, event.ID, sub.ID, res.Status) header := resp.Header() for h := range header { res.Headers[h] = header.Get(h) @@ -385,7 +430,7 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, } func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, event *core.EventDelivery, data core.DataArray, fastAck bool) { - req, res, gwErr := wh.attemptRequest(ctx, sub, event, data) + req, res, gwErr := wh.attemptRequest(ctx, sub, []*core.EventDelivery{event}, []core.DataArray{data}, false) if gwErr != nil { // Generate a bad-gateway error response - we always want to send something back, // rather than just causing timeouts @@ -444,108 +489,8 @@ func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, s } } -func (wh *WebHooks) attemptBatchRequest(ctx context.Context, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) (req *whRequest, res *whResponse, err error) { - withData := sub.Options.WithData != nil && *sub.Options.WithData - - allData := make([]*fftypes.JSONAny, 0, len(data)) - - if withData { - // We can either append all the data as flat map - // or nest it based on the event name? - // TODO ask as part of review if we want to support this - for _, d := range data { - for _, element := range d { - if element.Value != nil { - allData = append(allData, element.Value) - } - } - } - - // if len(allData) == 0 { - // firstData = fftypes.JSONObject{} - // } else { - // // Use JSONObjectOk instead of JSONObject - // // JSONObject fails for datatypes such as array, string, bool, number etc - // firstData, valid = allData[0].JSONObjectOk() - // if !valid { - // firstData = fftypes.JSONObject{ - // "value": allData[0], - // } - // } - // } - } - - client := wh.client - if sub.Options.RestyClient != nil { - client = sub.Options.RestyClient - } - - req, err = wh.buildRequest(ctx, client, sub.Options.TransportOptions(), nil) - if err != nil { - return nil, nil, err - } - - // Need to play around with these - if req.method == http.MethodPost || req.method == http.MethodPatch || req.method == http.MethodPut { - switch { - case req.body != nil: - // We might have been told to extract a body from the first data record - req.r.SetBody(req.body) - case len(allData) > 0: - // We've got an array of data to POST - // Send all the data of each request - req.r.SetBody(allData) - default: - // Just send the event itself - req.r.SetBody(events) - } - } - - resp, err := req.r.Execute(req.method, req.url) - if err != nil { - return nil, nil, err - } - defer func() { _ = resp.RawBody().Close() }() - - res = &whResponse{ - Status: resp.StatusCode(), - Headers: fftypes.JSONObject{}, - } - - header := resp.Header() - for h := range header { - res.Headers[h] = header.Get(h) - } - contentType := header.Get("Content-Type") - log.L(ctx).Debugf("Response content-type '%s' forceJSON=%t", contentType, req.forceJSON) - if req.forceJSON { - contentType = "application/json" - } - res.Headers["Content-Type"] = contentType - if req.forceJSON || strings.HasPrefix(contentType, "application/json") { - var resData interface{} - err = json.NewDecoder(resp.RawBody()).Decode(&resData) - if err != nil { - return nil, nil, i18n.WrapError(ctx, err, coremsgs.MsgWebhooksReplyBadJSON) - } - b, _ := json.Marshal(&resData) // we know we can re-marshal It - res.Body = fftypes.JSONAnyPtrBytes(b) - } else { - // Anything other than JSON, gets returned as a JSON string in base64 encoding - buf := &bytes.Buffer{} - buf.WriteByte('"') - b64Encoder := base64.NewEncoder(base64.StdEncoding, buf) - _, _ = io.Copy(b64Encoder, resp.RawBody()) - _ = b64Encoder.Close() - buf.WriteByte('"') - res.Body = fftypes.JSONAnyPtrBytes(buf.Bytes()) - } - - return req, res, nil -} - func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray, fastAck bool) { - req, res, gwErr := wh.attemptBatchRequest(ctx, sub, events, data) + req, res, gwErr := wh.attemptRequest(ctx, sub, events, data, true) if gwErr != nil { // Generate a bad-gateway error response - we always want to send something back, // rather than just causing timeouts diff --git a/internal/events/webhooks/webhooks_test.go b/internal/events/webhooks/webhooks_test.go index c303b583bf..21d3bfaf5e 100644 --- a/internal/events/webhooks/webhooks_test.go +++ b/internal/events/webhooks/webhooks_test.go @@ -1225,11 +1225,12 @@ func TestRequestWithBodyReplyEndToEndWithBatch(t *testing.T) { r.HandleFunc("/myapi", func(res http.ResponseWriter, req *http.Request) { assert.Equal(t, "myheaderval", req.Header.Get("My-Header")) assert.Equal(t, "myqueryval", req.URL.Query().Get("my-query")) - var data []fftypes.JSONObject + var data [][]fftypes.JSONAny err := json.NewDecoder(req.Body).Decode(&data) assert.NoError(t, err) assert.Equal(t, len(data), 2) - assert.Equal(t, "inputvalue", data[0].GetObject("in_body").GetString("inputfield")) + firstDataArray := data[0] + assert.Equal(t, "inputvalue", firstDataArray[0].JSONObject().GetObject("in_body").GetString("inputfield")) res.Header().Set("my-reply-header", "myheaderval2") res.WriteHeader(200) res.Write([]byte(`{ From d62b758f45a03d9d13ce07a740428208c3cbcab0 Mon Sep 17 00:00:00 2001 From: Enrique Lacal Date: Thu, 6 Jul 2023 21:24:15 +0100 Subject: [PATCH 04/10] one more refactor of body creation Signed-off-by: Enrique Lacal --- internal/events/webhooks/webhooks.go | 39 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index 1a93485917..abc8e05d1c 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -291,24 +291,29 @@ func (wh *WebHooks) buildBody(events []*core.EventDelivery, data []core.DataArra return nil, nil } - if withData { - // We are in the case of batching - // [[{"event1data1": "stuff"}],[{"event2data1": "stuff"}]] - // [[{"event1data1": "stuff"},{"event1data2":"stuff"}],[{"event2data1": "stuff"},{"event2data2":"stuff"}]] - if len(data) > 1 && batch { - allData := [][]*fftypes.JSONAny{} - for _, eventData := range data { - allEventData := []*fftypes.JSONAny{} - for _, d := range eventData { - if d.Value != nil { - allEventData = append(allEventData, d.Value) + if batch { + if withData { + if len(data) > 0 { + allData := [][]*fftypes.JSONAny{} + for _, eventData := range data { + allEventData := []*fftypes.JSONAny{} + for _, d := range eventData { + if d.Value != nil { + allEventData = append(allEventData, d.Value) + } } + allData = append(allData, allEventData) } - allData = append(allData, allEventData) + return allData, nil } - return allData, nil } + // [{"event1": "stuff"},"event2": "stuff"}] + // [{"event1": "stuff"}] + return events, nil + } + + if withData { if len(data) == 1 { eventData := []*fftypes.JSONAny{} for _, d := range data[0] { @@ -345,13 +350,7 @@ func (wh *WebHooks) buildBody(events []*core.EventDelivery, data []core.DataArra } } - // We haven't returned yet so we are not in data - // We only send the events - if batch { - // [{"event1": "stuff"},"event2": "stuff"}] - // [{"event1": "stuff"}] - body = events - } else if len(events) == 1 { + if body == nil { // {"event1": "stuff"} body = events[0] } From 74c4d5d4fc174829a898841d67c9ff924f7f72c4 Mon Sep 17 00:00:00 2001 From: Enrique Lacal Date: Thu, 6 Jul 2023 22:45:33 +0100 Subject: [PATCH 05/10] more refactoring the refactoring Signed-off-by: Enrique Lacal --- internal/events/webhooks/webhooks.go | 196 +++++++++++++++------- internal/events/webhooks/webhooks_test.go | 5 +- 2 files changed, 138 insertions(+), 63 deletions(-) diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index abc8e05d1c..94de1b32bd 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -286,82 +286,158 @@ func (wh *WebHooks) ValidateOptions(ctx context.Context, options *core.Subscript return err } -func (wh *WebHooks) buildBody(events []*core.EventDelivery, data []core.DataArray, withData bool, batch bool) (body interface{}, firstData fftypes.JSONObject) { - if len(events) == 0 && len(data) == 0 { - return nil, nil - } - - if batch { - if withData { - if len(data) > 0 { - allData := [][]*fftypes.JSONAny{} - for _, eventData := range data { - allEventData := []*fftypes.JSONAny{} - for _, d := range eventData { - if d.Value != nil { - allEventData = append(allEventData, d.Value) - } - } - allData = append(allData, allEventData) - } - return allData, nil +func (wh *WebHooks) buildBody(withData bool, event *core.EventDelivery, data core.DataArray) (body *fftypes.JSONAny, firstData fftypes.JSONObject, err error) { + allData := make([]*fftypes.JSONAny, 0, len(data)) + if withData { + for _, d := range data { + if d.Value != nil { + allData = append(allData, d.Value) } } - - // [{"event1": "stuff"},"event2": "stuff"}] - // [{"event1": "stuff"}] - return events, nil - } - - if withData { - if len(data) == 1 { - eventData := []*fftypes.JSONAny{} - for _, d := range data[0] { - if d.Value != nil { - eventData = append(eventData, d.Value) + if len(allData) == 0 { + firstData = fftypes.JSONObject{} + return fftypes.JSONAnyPtr("{}"), firstData, nil + } else { + // Use JSONObjectOk instead of JSONObject + // JSONObject fails for datatypes such as array, string, bool, number etc + var valid bool + firstData, valid = allData[0].JSONObjectOk() + if !valid { + firstData = fftypes.JSONObject{ + "value": allData[0], } } - if len(eventData) == 0 { - // Send an empty object if ask withData but no data available - firstData = fftypes.JSONObject{} - body = firstData - } - - if len(eventData) >= 1 { - // Use JSONObjectOk instead of JSONObject - // JSONObject fails for datatypes such as array, string, bool, number etc - var valid bool - firstData, valid = eventData[0].JSONObjectOk() - if !valid { - firstData = fftypes.JSONObject{ - "value": eventData[0], - } - } - - if len(eventData) == 1 { - body = firstData - } else { - body = eventData + if len(allData) == 1 { + encodedFirstData, err := json.Marshal(firstData) + if err != nil { + return nil, nil, err } + return fftypes.JSONAnyPtrBytes(encodedFirstData), firstData, nil } - - return body, firstData } + encodedData, err := json.Marshal(allData) + if err != nil { + return nil, nil, err + } + + return fftypes.JSONAnyPtrBytes(encodedData), firstData, nil } - if body == nil { - // {"event1": "stuff"} - body = events[0] + encodedEvent, err := json.Marshal(event) + if err != nil { + return nil, nil, err } - return body, firstData + return fftypes.JSONAnyPtrBytes(encodedEvent), nil, nil } +// func (wh *WebHooks) buildBody(events []*core.EventDelivery, data []core.DataArray, withData bool, batch bool) (body interface{}, firstData fftypes.JSONObject) { +// if len(events) == 0 && len(data) == 0 { +// return nil, nil +// } + +// myArray := []*fftypes.JSONAny{} + +// var foo *fftypes.JSONAny = myArray +// json + +// fftypes.JSONAnyPtrBytes(stuff) + +// if batch { +// if withData { +// if len(data) > 0 { +// allData := [][]*fftypes.JSONAny{} +// for _, eventData := range data { +// allEventData := []*fftypes.JSONAny{} +// for _, d := range eventData { +// if d.Value != nil { +// allEventData = append(allEventData, d.Value) +// } +// } +// allData = append(allData, allEventData) +// } +// return allData, nil +// } +// } + +// // [{"event1": "stuff"},"event2": "stuff"}] +// // [{"event1": "stuff"}] +// return events, nil +// } + +// if withData { +// if len(data) == 1 { +// eventData := []*fftypes.JSONAny{} +// for _, d := range data[0] { +// if d.Value != nil { +// eventData = append(eventData, d.Value) +// } +// } + +// if len(eventData) == 0 { +// // Send an empty object if ask withData but no data available +// firstData = fftypes.JSONObject{} +// body = firstData +// } + +// if len(eventData) >= 1 { +// // Use JSONObjectOk instead of JSONObject +// // JSONObject fails for datatypes such as array, string, bool, number etc +// var valid bool +// firstData, valid = eventData[0].JSONObjectOk() +// if !valid { +// firstData = fftypes.JSONObject{ +// "value": eventData[0], +// } +// } + +// if len(eventData) == 1 { +// body = firstData +// } else { +// body = eventData +// } +// } + +// return body, firstData +// } +// } + +// if body == nil && len(events) > 0 { +// // {"event1": "stuff"} +// body = events[0] +// } + +// return body, firstData +// } + func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray, batch bool) (req *whRequest, res *whResponse, err error) { withData := sub.Options.WithData != nil && *sub.Options.WithData - body, firstData := wh.buildBody(events, data, withData, batch) + var body *fftypes.JSONAny + var firstData fftypes.JSONObject + if len(events) == 1 && len(data) == 1 { + body, firstData, err = wh.buildBody(withData, events[0], data[0]) + if err != nil { + return nil, nil, err + } + } else { + batchBody := []*fftypes.JSONAny{} + for i := 0; i < len(events); i++ { + eventBody, _, err := wh.buildBody(withData, events[i], data[i]) + if err != nil { + return nil, nil, err + } + batchBody = append(batchBody, eventBody) + } + + encodedBody, err := json.Marshal(batchBody) + if err != nil { + return nil, nil, err + } + + body = fftypes.JSONAnyPtrBytes(encodedBody) + } client := wh.client if sub.Options.RestyClient != nil { @@ -379,7 +455,7 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, // We might have been told to extract a body from the first data record req.r.SetBody(req.body) default: - req.r.SetBody(body) + req.r.SetBody(body.Bytes()) } } diff --git a/internal/events/webhooks/webhooks_test.go b/internal/events/webhooks/webhooks_test.go index 21d3bfaf5e..c303b583bf 100644 --- a/internal/events/webhooks/webhooks_test.go +++ b/internal/events/webhooks/webhooks_test.go @@ -1225,12 +1225,11 @@ func TestRequestWithBodyReplyEndToEndWithBatch(t *testing.T) { r.HandleFunc("/myapi", func(res http.ResponseWriter, req *http.Request) { assert.Equal(t, "myheaderval", req.Header.Get("My-Header")) assert.Equal(t, "myqueryval", req.URL.Query().Get("my-query")) - var data [][]fftypes.JSONAny + var data []fftypes.JSONObject err := json.NewDecoder(req.Body).Decode(&data) assert.NoError(t, err) assert.Equal(t, len(data), 2) - firstDataArray := data[0] - assert.Equal(t, "inputvalue", firstDataArray[0].JSONObject().GetObject("in_body").GetString("inputfield")) + assert.Equal(t, "inputvalue", data[0].GetObject("in_body").GetString("inputfield")) res.Header().Set("my-reply-header", "myheaderval2") res.WriteHeader(200) res.Write([]byte(`{ From c055e6b0578e00762e2728d17fb4202e5459b440 Mon Sep 17 00:00:00 2001 From: Enrique Lacal Date: Thu, 6 Jul 2023 23:32:27 +0100 Subject: [PATCH 06/10] fix: creating of body and check withData with batch are incompatible Signed-off-by: Enrique Lacal --- internal/coremsgs/en_error_messages.go | 1 + internal/events/event_dispatcher.go | 16 +-- internal/events/event_dispatcher_test.go | 6 +- internal/events/system/events.go | 2 +- internal/events/system/events_test.go | 2 +- internal/events/webhooks/webhooks.go | 131 ++++-------------- internal/events/webhooks/webhooks_test.go | 2 +- internal/events/websockets/websockets.go | 2 +- internal/events/websockets/websockets_test.go | 2 +- internal/orchestrator/subscriptions.go | 4 + mocks/eventsmocks/plugin.go | 10 +- pkg/core/event.go | 7 +- pkg/events/plugin.go | 2 +- 13 files changed, 60 insertions(+), 127 deletions(-) diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go index b761a292aa..03e31a50fe 100644 --- a/internal/coremsgs/en_error_messages.go +++ b/internal/coremsgs/en_error_messages.go @@ -292,4 +292,5 @@ var ( MsgInvalidMessageIdentity = ffe("FF10453", "Invalid message '%s'. Author '%s' does not match identity registered to %s: %s (%s)") MsgDuplicateTLSConfig = ffe("FF10454", "Found duplicate TLS Config '%s'", 400) MsgNotFoundTLSConfig = ffe("FF10455", "Provided TLS Config name '%s' not found for namespace '%s'", 400) + MsgBatchWithDataNotSupport = ffe("FF10456", "Provided subscription '%s' enables batching and withData which is not support", 400) ) diff --git a/internal/events/event_dispatcher.go b/internal/events/event_dispatcher.go index 51f6a0f4e1..8a4578b0a9 100644 --- a/internal/events/event_dispatcher.go +++ b/internal/events/event_dispatcher.go @@ -383,8 +383,7 @@ func (ed *eventDispatcher) handleAckOffsetUpdate(ack ackNack) { func (ed *eventDispatcher) deliverBatchedEvents() { withData := ed.subscription.definition.Options.WithData != nil && *ed.subscription.definition.Options.WithData - var events []*core.EventDelivery - var dataSet []core.DataArray + var events []*core.CombinedEventDataDelivery var batchTimeoutContext context.Context var batchTimeoutCancel func() for { @@ -404,23 +403,21 @@ func (ed *eventDispatcher) deliverBatchedEvents() { return } - if events == nil && dataSet == nil { - events = []*core.EventDelivery{} - dataSet = []core.DataArray{} + if events == nil { + events = []*core.CombinedEventDataDelivery{} batchTimeoutContext, batchTimeoutCancel = context.WithTimeout(ed.ctx, ed.batchTimeout) } log.L(ed.ctx).Debugf("Dispatching %s event in a batch: %.10d/%s [%s]: ref=%s/%s", ed.transport.Name(), event.Sequence, event.ID, event.Type, event.Namespace, event.Reference) - events = append(events, event) - var data []*core.Data var err error if withData && event.Message != nil { data, _, err = ed.data.GetMessageDataCached(ed.ctx, event.Message) - dataSet = append(dataSet, data) } + events = append(events, &core.CombinedEventDataDelivery{Event: event, Data: data}) + if err != nil { ed.deliveryResponse(&core.EventDeliveryResponse{ID: event.ID, Rejected: true}) } @@ -435,10 +432,9 @@ func (ed *eventDispatcher) deliverBatchedEvents() { } if len(events) == ed.readAhead || (timedOut && len(events) > 0) { - _ = ed.transport.BatchDeliveryRequest(ed.ctx, ed.connID, ed.subscription.definition, events, dataSet) + _ = ed.transport.BatchDeliveryRequest(ed.ctx, ed.connID, ed.subscription.definition, events) // If err handle all the delivery responses for all the events?? events = nil - dataSet = nil } } } diff --git a/internal/events/event_dispatcher_test.go b/internal/events/event_dispatcher_test.go index 7060d66118..7368c6365e 100644 --- a/internal/events/event_dispatcher_test.go +++ b/internal/events/event_dispatcher_test.go @@ -1176,11 +1176,11 @@ func TestEventDispatcherBatchReadAhead(t *testing.T) { mdm := ed.data.(*datamocks.Manager) eventDeliveries := make(chan *core.EventDelivery) - deliveryRequestMock := mei.On("BatchDeliveryRequest", ed.ctx, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + deliveryRequestMock := mei.On("BatchDeliveryRequest", ed.ctx, mock.Anything, mock.Anything, mock.Anything).Return(nil) deliveryRequestMock.RunFn = func(a mock.Arguments) { - batchEvents := a.Get(3).([]*core.EventDelivery) + batchEvents := a.Get(3).([]*core.CombinedEventDataDelivery) for _, event := range batchEvents { - eventDeliveries <- event + eventDeliveries <- event.Event } } diff --git a/internal/events/system/events.go b/internal/events/system/events.go index 6dbecb3060..484b577944 100644 --- a/internal/events/system/events.go +++ b/internal/events/system/events.go @@ -134,7 +134,7 @@ func (se *Events) DeliveryRequest(ctx context.Context, connID string, sub *core. return nil } -func (se *Events) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { +func (se *Events) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.CombinedEventDataDelivery) error { return nil } diff --git a/internal/events/system/events_test.go b/internal/events/system/events_test.go index d85727bec3..9364f1255f 100644 --- a/internal/events/system/events_test.go +++ b/internal/events/system/events_test.go @@ -153,5 +153,5 @@ func TestEventDeliveryBatch(t *testing.T) { }, } - se.BatchDeliveryRequest(se.ctx, "id", sub, []*core.EventDelivery{}, []core.DataArray{}) + se.BatchDeliveryRequest(se.ctx, "id", sub, []*core.CombinedEventDataDelivery{}) } diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index 94de1b32bd..1ad4e1792f 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -286,17 +286,16 @@ func (wh *WebHooks) ValidateOptions(ctx context.Context, options *core.Subscript return err } -func (wh *WebHooks) buildBody(withData bool, event *core.EventDelivery, data core.DataArray) (body *fftypes.JSONAny, firstData fftypes.JSONObject, err error) { - allData := make([]*fftypes.JSONAny, 0, len(data)) - if withData { - for _, d := range data { +func (wh *WebHooks) buildBody(withData bool, combinedEvent *core.CombinedEventDataDelivery) (body *fftypes.JSONAny, firstData fftypes.JSONObject, err error) { + allData := make([]*fftypes.JSONAny, 0, len(combinedEvent.Data)) + if withData && len(combinedEvent.Data) > 0 { + for _, d := range combinedEvent.Data { if d.Value != nil { allData = append(allData, d.Value) } } if len(allData) == 0 { firstData = fftypes.JSONObject{} - return fftypes.JSONAnyPtr("{}"), firstData, nil } else { // Use JSONObjectOk instead of JSONObject // JSONObject fails for datatypes such as array, string, bool, number etc @@ -324,107 +323,29 @@ func (wh *WebHooks) buildBody(withData bool, event *core.EventDelivery, data cor return fftypes.JSONAnyPtrBytes(encodedData), firstData, nil } - encodedEvent, err := json.Marshal(event) + firstData = fftypes.JSONObject{} + encodedEvent, err := json.Marshal(combinedEvent.Event) if err != nil { return nil, nil, err } - return fftypes.JSONAnyPtrBytes(encodedEvent), nil, nil + return fftypes.JSONAnyPtrBytes(encodedEvent), firstData, nil } -// func (wh *WebHooks) buildBody(events []*core.EventDelivery, data []core.DataArray, withData bool, batch bool) (body interface{}, firstData fftypes.JSONObject) { -// if len(events) == 0 && len(data) == 0 { -// return nil, nil -// } - -// myArray := []*fftypes.JSONAny{} - -// var foo *fftypes.JSONAny = myArray -// json - -// fftypes.JSONAnyPtrBytes(stuff) - -// if batch { -// if withData { -// if len(data) > 0 { -// allData := [][]*fftypes.JSONAny{} -// for _, eventData := range data { -// allEventData := []*fftypes.JSONAny{} -// for _, d := range eventData { -// if d.Value != nil { -// allEventData = append(allEventData, d.Value) -// } -// } -// allData = append(allData, allEventData) -// } -// return allData, nil -// } -// } - -// // [{"event1": "stuff"},"event2": "stuff"}] -// // [{"event1": "stuff"}] -// return events, nil -// } - -// if withData { -// if len(data) == 1 { -// eventData := []*fftypes.JSONAny{} -// for _, d := range data[0] { -// if d.Value != nil { -// eventData = append(eventData, d.Value) -// } -// } - -// if len(eventData) == 0 { -// // Send an empty object if ask withData but no data available -// firstData = fftypes.JSONObject{} -// body = firstData -// } - -// if len(eventData) >= 1 { -// // Use JSONObjectOk instead of JSONObject -// // JSONObject fails for datatypes such as array, string, bool, number etc -// var valid bool -// firstData, valid = eventData[0].JSONObjectOk() -// if !valid { -// firstData = fftypes.JSONObject{ -// "value": eventData[0], -// } -// } - -// if len(eventData) == 1 { -// body = firstData -// } else { -// body = eventData -// } -// } - -// return body, firstData -// } -// } - -// if body == nil && len(events) > 0 { -// // {"event1": "stuff"} -// body = events[0] -// } - -// return body, firstData -// } - -func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray, batch bool) (req *whRequest, res *whResponse, err error) { +func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, events []*core.CombinedEventDataDelivery, batch bool) (req *whRequest, res *whResponse, err error) { withData := sub.Options.WithData != nil && *sub.Options.WithData var body *fftypes.JSONAny var firstData fftypes.JSONObject - if len(events) == 1 && len(data) == 1 { - body, firstData, err = wh.buildBody(withData, events[0], data[0]) + if len(events) == 1 && !batch { + body, firstData, err = wh.buildBody(withData, events[0]) if err != nil { return nil, nil, err } } else { batchBody := []*fftypes.JSONAny{} - for i := 0; i < len(events); i++ { - eventBody, _, err := wh.buildBody(withData, events[i], data[i]) + for _, event := range events { + eventBody, _, err := wh.buildBody(withData, event) if err != nil { return nil, nil, err } @@ -505,7 +426,7 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, } func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, event *core.EventDelivery, data core.DataArray, fastAck bool) { - req, res, gwErr := wh.attemptRequest(ctx, sub, []*core.EventDelivery{event}, []core.DataArray{data}, false) + req, res, gwErr := wh.attemptRequest(ctx, sub, []*core.CombinedEventDataDelivery{{Event: event, Data: data}}, false) if gwErr != nil { // Generate a bad-gateway error response - we always want to send something back, // rather than just causing timeouts @@ -564,8 +485,8 @@ func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, s } } -func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray, fastAck bool) { - req, res, gwErr := wh.attemptRequest(ctx, sub, events, data, true) +func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, events []*core.CombinedEventDataDelivery, fastAck bool) { + req, res, gwErr := wh.attemptRequest(ctx, sub, events, true) if gwErr != nil { // Generate a bad-gateway error response - we always want to send something back, // rather than just causing timeouts @@ -585,8 +506,12 @@ func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply log.L(wh.ctx).Tracef("Webhook response: %s", string(b)) // For each event emit a response - for _, event := range events { + for _, combinedEvent := range events { + event := combinedEvent.Event // Emit the response + if event == nil { + continue + } if reply && event.Message != nil { txType := fftypes.FFEnum(strings.ToLower(sub.Options.TransportOptions().GetString("replytx"))) if req != nil && req.replyTx != "" { @@ -665,11 +590,12 @@ func (wh *WebHooks) DeliveryRequest(ctx context.Context, connID string, sub *cor return nil } -func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { +func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.CombinedEventDataDelivery) error { reply := sub.Options.TransportOptions().GetBool("reply") if reply { - nonReplyEvents := []*core.EventDelivery{} - for _, event := range events { + nonReplyEvents := []*core.CombinedEventDataDelivery{} + for _, combinedEvent := range events { + event := combinedEvent.Event // We cowardly refuse to dispatch a message that is itself a reply, as it's hard for users to // avoid loops - and there's no way for us to detect here if a user has configured correctly // to avoid a loop. @@ -685,7 +611,7 @@ func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub continue } - nonReplyEvents = append(nonReplyEvents, event) + nonReplyEvents = append(nonReplyEvents, combinedEvent) } // Override the events to send without the reply events events = nonReplyEvents @@ -695,7 +621,8 @@ func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub // NOTE: We cannot use this with reply mode, as when we're sending a reply the `DeliveryResponse` // callback must include the reply in-line. if !reply && sub.Options.TransportOptions().GetBool("fastack") { - for _, event := range events { + for _, combinedEvent := range events { + event := combinedEvent.Event if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ ID: event.ID, @@ -704,11 +631,11 @@ func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub }) } } - go wh.doBatchedDelivery(ctx, connID, reply, sub, events, data, true) + go wh.doBatchedDelivery(ctx, connID, reply, sub, events, true) return nil } - wh.doBatchedDelivery(ctx, connID, reply, sub, events, data, false) + wh.doBatchedDelivery(ctx, connID, reply, sub, events, false) return nil } diff --git a/internal/events/webhooks/webhooks_test.go b/internal/events/webhooks/webhooks_test.go index c303b583bf..c326b862b7 100644 --- a/internal/events/webhooks/webhooks_test.go +++ b/internal/events/webhooks/webhooks_test.go @@ -1351,7 +1351,7 @@ func TestRequestWithBodyReplyEndToEndWithBatch(t *testing.T) { return true })).Return(nil) - err := wh.BatchDeliveryRequest(wh.ctx, mock.Anything, sub, []*core.EventDelivery{event1, event2}, []core.DataArray{data1, data2}) + err := wh.BatchDeliveryRequest(wh.ctx, mock.Anything, sub, []*core.CombinedEventDataDelivery{{Event: event1, Data: data1}, {Event: event2, Data: data2}}) assert.NoError(t, err) mcb.AssertExpectations(t) diff --git a/internal/events/websockets/websockets.go b/internal/events/websockets/websockets.go index e989164e17..b7c210ed70 100644 --- a/internal/events/websockets/websockets.go +++ b/internal/events/websockets/websockets.go @@ -218,6 +218,6 @@ func (ws *WebSockets) GetStatus() *core.WebSocketStatus { return status } -func (ws *WebSockets) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.EventDelivery, data []core.DataArray) error { +func (ws *WebSockets) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.CombinedEventDataDelivery) error { return nil } diff --git a/internal/events/websockets/websockets_test.go b/internal/events/websockets/websockets_test.go index 471b849d4c..2ef36ca2a0 100644 --- a/internal/events/websockets/websockets_test.go +++ b/internal/events/websockets/websockets_test.go @@ -817,5 +817,5 @@ func TestEventDeliveryBatch(t *testing.T) { }, } - ws.BatchDeliveryRequest(ws.ctx, "id", sub, []*core.EventDelivery{}, []core.DataArray{}) + ws.BatchDeliveryRequest(ws.ctx, "id", sub, []*core.CombinedEventDataDelivery{}) } diff --git a/internal/orchestrator/subscriptions.go b/internal/orchestrator/subscriptions.go index 8f1989b1a4..08b9dc7574 100644 --- a/internal/orchestrator/subscriptions.go +++ b/internal/orchestrator/subscriptions.go @@ -64,6 +64,10 @@ func (or *orchestrator) createUpdateSubscription(ctx context.Context, subDef *co } } + if subDef.Options.Batch && subDef.Options.WithData != nil && *subDef.Options.WithData { + return nil, i18n.NewError(ctx, coremsgs.MsgBatchWithDataNotSupport, subDef.Name) + } + return subDef, or.events.CreateUpdateDurableSubscription(ctx, subDef, mustNew) } diff --git a/mocks/eventsmocks/plugin.go b/mocks/eventsmocks/plugin.go index 8ef7a2c124..9ce4f3c917 100644 --- a/mocks/eventsmocks/plugin.go +++ b/mocks/eventsmocks/plugin.go @@ -21,13 +21,13 @@ type Plugin struct { mock.Mock } -// BatchDeliveryRequest provides a mock function with given fields: ctx, connID, sub, event, data -func (_m *Plugin) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error { - ret := _m.Called(ctx, connID, sub, event, data) +// BatchDeliveryRequest provides a mock function with given fields: ctx, connID, sub, _a3 +func (_m *Plugin) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, _a3 []*core.CombinedEventDataDelivery) error { + ret := _m.Called(ctx, connID, sub, _a3) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, *core.Subscription, []*core.EventDelivery, []core.DataArray) error); ok { - r0 = rf(ctx, connID, sub, event, data) + if rf, ok := ret.Get(0).(func(context.Context, string, *core.Subscription, []*core.CombinedEventDataDelivery) error); ok { + r0 = rf(ctx, connID, sub, _a3) } else { r0 = ret.Error(0) } diff --git a/pkg/core/event.go b/pkg/core/event.go index b96e2d7263..6f659be681 100644 --- a/pkg/core/event.go +++ b/pkg/core/event.go @@ -1,4 +1,4 @@ -// Copyright © 2022 Kaleido, Inc. +// Copyright © 2023 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -99,6 +99,11 @@ type EventDelivery struct { Subscription SubscriptionRef `json:"subscription"` } +type CombinedEventDataDelivery struct { + Event *EventDelivery + Data DataArray +} + // EventDeliveryResponse is the payload an application sends back, to confirm it has accepted (or rejected) the event and as such // does not need to receive it again. type EventDeliveryResponse struct { diff --git a/pkg/events/plugin.go b/pkg/events/plugin.go index 3b10be84ef..386c955274 100644 --- a/pkg/events/plugin.go +++ b/pkg/events/plugin.go @@ -53,7 +53,7 @@ type Plugin interface { // DeliveryBatchRequest requests delivery of multiple events on a connection, which must later be responded to // Data will only be supplied as non-nil if the subscription is set to include data - BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event []*core.EventDelivery, data []core.DataArray) error + BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.CombinedEventDataDelivery) error // NamespaceRestarted is called after a namespace restarts. For a connect-in style plugin, like // WebSockets, this must re-register any active connections that started before the time passed in. From eb028bd74804a890d65195432c7249d7687c34af Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 6 Jul 2023 23:13:48 -0400 Subject: [PATCH 07/10] Tweaks to processing to simplfy and deferring parsing Signed-off-by: Peter Broadhurst --- go.sum | 1 + internal/events/webhooks/webhooks.go | 193 ++++++++++++++------------- 2 files changed, 100 insertions(+), 94 deletions(-) diff --git a/go.sum b/go.sum index aad9968cb0..8f102487d6 100644 --- a/go.sum +++ b/go.sum @@ -447,6 +447,7 @@ github.com/go-latex/latex v0.0.0-20210118124228-b3d85cf34e07/go.mod h1:CO1AlKB2C github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= +github.com/go-logfmt/logfmt v0.5.1/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs= github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index 1ad4e1792f..6538533c05 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -57,11 +57,17 @@ type whRequest struct { r *resty.Request url string method string - body fftypes.JSONObject forceJSON bool replyTx string } +type whPayload struct { + input fftypes.JSONObject + parsedData0 fftypes.JSONObject + data0 *fftypes.JSONAny + body interface{} +} + type whResponse struct { Status int `json:"status"` Headers fftypes.JSONObject `json:"headers"` @@ -109,7 +115,73 @@ func (wh *WebHooks) Capabilities() *events.Capabilities { return wh.capabilities } -func (wh *WebHooks) buildRequest(ctx context.Context, restyClient *resty.Client, options fftypes.JSONObject, firstData fftypes.JSONObject) (req *whRequest, err error) { +// firstData parses data0 from the data the first time it's needed, and guarantees a non-nil result +func (p *whPayload) firstData() fftypes.JSONObject { + if p.parsedData0 != nil { + return p.parsedData0 + } + if p.data0 == nil { + p.parsedData0 = fftypes.JSONObject{} + } else { + // Use JSONObjectOk instead of JSONObject + // JSONObject fails for datatypes such as array, string, bool, number etc + var valid bool + p.parsedData0, valid = p.data0.JSONObjectOk() + if !valid { + p.parsedData0 = fftypes.JSONObject{ + "value": p.parsedData0, + } + } + } + return p.parsedData0 +} + +func (wh *WebHooks) buildPayload(ctx context.Context, sub *core.Subscription, event *core.CombinedEventDataDelivery) *whPayload { + + withData := sub.Options.WithData != nil && *sub.Options.WithData + options := sub.Options.TransportOptions() + p := &whPayload{ + // Options on how to process the input + input: options.GetObject("input"), + } + + allData := make([]*fftypes.JSONAny, 0, len(event.Data)) + if withData { + for _, d := range event.Data { + if d.Value != nil { + allData = append(allData, d.Value) + if p.data0 == nil { + p.data0 = d.Value + } + } + } + } + + // Choose to sub-select a field to send as the body + var bodyFromFirstData *fftypes.JSONAny + inputBody := p.input.GetString("body") + if inputBody != "" { + bodyFromFirstData = fftypes.JSONAnyPtr(p.firstData().GetObject(inputBody).String()) + } + + switch { + case bodyFromFirstData != nil: + // We might have been told to extract a body from the first data record + p.body = bodyFromFirstData.String() + case len(allData) > 1: + // We've got an array of data to POST + p.body = allData + case len(allData) == 1: + // Just send the first object, forced into an object per the rules in firstData() + p.body = p.firstData() + default: + // Just send the event itself + p.body = event.Event + } + return p +} + +func (wh *WebHooks) buildRequest(ctx context.Context, restyClient *resty.Client, options fftypes.JSONObject, p *whPayload) (req *whRequest, err error) { req = &whRequest{ r: restyClient.R(). SetDoNotParseResponse(true). @@ -145,34 +217,28 @@ func (wh *WebHooks) buildRequest(ctx context.Context, restyClient *resty.Client, } _ = req.r.SetQueryParam(q, s) } - if firstData != nil { - // Options on how to process the input - input := options.GetObject("input") + // p will be nil for a batch delivery + if p != nil { // Dynamic query support from input - inputQuery := input.GetString("query") + inputQuery := p.input.GetString("query") if inputQuery != "" { - iq := firstData.GetObject(inputQuery) + iq := p.firstData().GetObject(inputQuery) for q := range iq { _ = req.r.SetQueryParam(q, iq.GetString(q)) } } // Dynamic header support from input - inputHeaders := input.GetString("headers") + inputHeaders := p.input.GetString("headers") if inputHeaders != "" { - ih := firstData.GetObject(inputHeaders) + ih := p.firstData().GetObject(inputHeaders) for h := range ih { _ = req.r.SetHeader(h, ih.GetString(h)) } } - // Choose to sub-select a field to send as the body - inputBody := input.GetString("body") - if inputBody != "" { - req.body = firstData.GetObject(inputBody) - } // Choose to add an additional dynamic path - inputPath := input.GetString("path") + inputPath := p.input.GetString("path") if inputPath != "" { - extraPath := strings.TrimPrefix(firstData.GetString(inputPath), "/") + extraPath := strings.TrimPrefix(p.firstData().GetString(inputPath), "/") if len(extraPath) > 0 { pathSegments := strings.Split(extraPath, "/") for _, ps := range pathSegments { @@ -181,9 +247,9 @@ func (wh *WebHooks) buildRequest(ctx context.Context, restyClient *resty.Client, } } // Choose to add an additional dynamic path - inputTxtype := input.GetString("replytx") + inputTxtype := p.input.GetString("replytx") if inputTxtype != "" { - txType := firstData.GetString(inputTxtype) + txType := p.firstData().GetString(inputTxtype) if len(txType) > 0 { req.replyTx = txType if strings.EqualFold(txType, "true") { @@ -282,82 +348,27 @@ func (wh *WebHooks) ValidateOptions(ctx context.Context, options *core.Subscript // So these clients should live as long as the plugin exists options.RestyClient = ffresty.NewWithConfig(wh.ctx, newFFRestyConfig) - _, err := wh.buildRequest(ctx, options.RestyClient, options.TransportOptions(), fftypes.JSONObject{}) + _, err := wh.buildRequest(ctx, options.RestyClient, options.TransportOptions(), nil) return err } -func (wh *WebHooks) buildBody(withData bool, combinedEvent *core.CombinedEventDataDelivery) (body *fftypes.JSONAny, firstData fftypes.JSONObject, err error) { - allData := make([]*fftypes.JSONAny, 0, len(combinedEvent.Data)) - if withData && len(combinedEvent.Data) > 0 { - for _, d := range combinedEvent.Data { - if d.Value != nil { - allData = append(allData, d.Value) - } - } - if len(allData) == 0 { - firstData = fftypes.JSONObject{} - } else { - // Use JSONObjectOk instead of JSONObject - // JSONObject fails for datatypes such as array, string, bool, number etc - var valid bool - firstData, valid = allData[0].JSONObjectOk() - if !valid { - firstData = fftypes.JSONObject{ - "value": allData[0], - } - } - - if len(allData) == 1 { - encodedFirstData, err := json.Marshal(firstData) - if err != nil { - return nil, nil, err - } - return fftypes.JSONAnyPtrBytes(encodedFirstData), firstData, nil - } - } - encodedData, err := json.Marshal(allData) - if err != nil { - return nil, nil, err - } - - return fftypes.JSONAnyPtrBytes(encodedData), firstData, nil - } - - firstData = fftypes.JSONObject{} - encodedEvent, err := json.Marshal(combinedEvent.Event) - if err != nil { - return nil, nil, err - } - - return fftypes.JSONAnyPtrBytes(encodedEvent), firstData, nil -} - func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, events []*core.CombinedEventDataDelivery, batch bool) (req *whRequest, res *whResponse, err error) { - withData := sub.Options.WithData != nil && *sub.Options.WithData - var body *fftypes.JSONAny - var firstData fftypes.JSONObject + var payloadForBuildingRequest *whPayload // only set for a single event delivery + var requestBody interface{} if len(events) == 1 && !batch { - body, firstData, err = wh.buildBody(withData, events[0]) - if err != nil { - return nil, nil, err - } + payloadForBuildingRequest = wh.buildPayload(ctx, sub, events[0]) + // Payload for POST/PATCH/PUT is what is calculated for a single event in buildPayload + requestBody = payloadForBuildingRequest.body } else { - batchBody := []*fftypes.JSONAny{} - for _, event := range events { - eventBody, _, err := wh.buildBody(withData, event) - if err != nil { - return nil, nil, err - } - batchBody = append(batchBody, eventBody) + batchBody := make([]interface{}, len(events)) + for i, event := range events { + // We only use the body itself from the whPayload - then discard it. + p := wh.buildPayload(ctx, sub, event) + batchBody[i] = p.body } - - encodedBody, err := json.Marshal(batchBody) - if err != nil { - return nil, nil, err - } - - body = fftypes.JSONAnyPtrBytes(encodedBody) + // Payload for POST/PATCH/PUT is the array of outputs calculated for a each event in buildPayload + requestBody = batchBody } client := wh.client @@ -365,19 +376,13 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, client = sub.Options.RestyClient } - req, err = wh.buildRequest(ctx, client, sub.Options.TransportOptions(), firstData) + req, err = wh.buildRequest(ctx, client, sub.Options.TransportOptions(), payloadForBuildingRequest) if err != nil { return nil, nil, err } if req.method == http.MethodPost || req.method == http.MethodPatch || req.method == http.MethodPut { - switch { - case req.body != nil: - // We might have been told to extract a body from the first data record - req.r.SetBody(req.body) - default: - req.r.SetBody(body.Bytes()) - } + req.r.SetBody(requestBody) } // log.L(wh.ctx).Debugf("Webhook-> %s %s event %s on subscription %s", req.method, req.url, event.ID, sub.ID) From 4ef4f39e16b9204bff6821fea9c30ef048db4e56 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 6 Jul 2023 23:39:44 -0400 Subject: [PATCH 08/10] Additional removal of code duplication and test coverage Signed-off-by: Peter Broadhurst --- internal/events/webhooks/webhooks.go | 77 ++------------- internal/events/webhooks/webhooks_test.go | 113 +++++++++++++++++++++- 2 files changed, 120 insertions(+), 70 deletions(-) diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index 6538533c05..d2c8185dd2 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -430,68 +430,8 @@ func (wh *WebHooks) attemptRequest(ctx context.Context, sub *core.Subscription, return req, res, nil } -func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, event *core.EventDelivery, data core.DataArray, fastAck bool) { - req, res, gwErr := wh.attemptRequest(ctx, sub, []*core.CombinedEventDataDelivery{{Event: event, Data: data}}, false) - if gwErr != nil { - // Generate a bad-gateway error response - we always want to send something back, - // rather than just causing timeouts - log.L(wh.ctx).Errorf("Failed to invoke webhook: %s", gwErr) - b, _ := json.Marshal(&fftypes.RESTError{ - Error: gwErr.Error(), - }) - res = &whResponse{ - Status: http.StatusBadGateway, - Headers: fftypes.JSONObject{ - "Content-Type": "application/json", - }, - Body: fftypes.JSONAnyPtrBytes(b), - } - } - b, _ := json.Marshal(&res) - log.L(wh.ctx).Tracef("Webhook response: %s", string(b)) - - // Emit the response - if reply && event.Message != nil { - txType := fftypes.FFEnum(strings.ToLower(sub.Options.TransportOptions().GetString("replytx"))) - if req != nil && req.replyTx != "" { - txType = fftypes.FFEnum(strings.ToLower(req.replyTx)) - } - if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { - log.L(wh.ctx).Debugf("Sending reply message for %s CID=%s", event.ID, event.Message.Header.ID) - cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ - ID: event.ID, - Rejected: false, - Subscription: event.Subscription, - Reply: &core.MessageInOut{ - Message: core.Message{ - Header: core.MessageHeader{ - CID: event.Message.Header.ID, - Group: event.Message.Header.Group, - Type: event.Message.Header.Type, - Topics: event.Message.Header.Topics, - Tag: sub.Options.TransportOptions().GetString("replytag"), - TxType: txType, - }, - }, - InlineData: core.InlineData{ - {Value: fftypes.JSONAnyPtrBytes(b)}, - }, - }, - }) - } - } else if !fastAck { - if cb, ok := wh.callbacks.handlers[sub.Namespace]; ok { - cb.DeliveryResponse(connID, &core.EventDeliveryResponse{ - ID: event.ID, - Rejected: false, - Subscription: event.Subscription, - }) - } - } -} - -func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, events []*core.CombinedEventDataDelivery, fastAck bool) { - req, res, gwErr := wh.attemptRequest(ctx, sub, events, true) +func (wh *WebHooks) doDelivery(ctx context.Context, connID string, reply bool, sub *core.Subscription, events []*core.CombinedEventDataDelivery, fastAck, batched bool) { + req, res, gwErr := wh.attemptRequest(ctx, sub, events, batched) if gwErr != nil { // Generate a bad-gateway error response - we always want to send something back, // rather than just causing timeouts @@ -514,9 +454,6 @@ func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply for _, combinedEvent := range events { event := combinedEvent.Event // Emit the response - if event == nil { - continue - } if reply && event.Message != nil { txType := fftypes.FFEnum(strings.ToLower(sub.Options.TransportOptions().GetString("replytx"))) if req != nil && req.replyTx != "" { @@ -555,7 +492,9 @@ func (wh *WebHooks) doBatchedDelivery(ctx context.Context, connID string, reply } } } + } + func (wh *WebHooks) DeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, event *core.EventDelivery, data core.DataArray) error { reply := sub.Options.TransportOptions().GetBool("reply") if reply && event.Message != nil && event.Message.Header.CID != nil { @@ -584,14 +523,14 @@ func (wh *WebHooks) DeliveryRequest(ctx context.Context, connID string, sub *cor Subscription: event.Subscription, }) } - go wh.doDelivery(ctx, connID, reply, sub, event, data, true) + go wh.doDelivery(ctx, connID, reply, sub, []*core.CombinedEventDataDelivery{{Event: event, Data: data}}, true, false) return nil } // NOTE: We could check here for batching and accumulate but we can't return because this causes the offset to jump... // TODO we don't look at the error here? - wh.doDelivery(ctx, connID, reply, sub, event, data, false) + wh.doDelivery(ctx, connID, reply, sub, []*core.CombinedEventDataDelivery{{Event: event, Data: data}}, false, false) return nil } @@ -636,11 +575,11 @@ func (wh *WebHooks) BatchDeliveryRequest(ctx context.Context, connID string, sub }) } } - go wh.doBatchedDelivery(ctx, connID, reply, sub, events, true) + go wh.doDelivery(ctx, connID, reply, sub, events, true, true) return nil } - wh.doBatchedDelivery(ctx, connID, reply, sub, events, false) + wh.doDelivery(ctx, connID, reply, sub, events, false, true) return nil } diff --git a/internal/events/webhooks/webhooks_test.go b/internal/events/webhooks/webhooks_test.go index c326b862b7..e6a5fdc90a 100644 --- a/internal/events/webhooks/webhooks_test.go +++ b/internal/events/webhooks/webhooks_test.go @@ -1069,7 +1069,7 @@ func TestRequestReplyDataArrayError(t *testing.T) { mcb.AssertExpectations(t) } -func TestWebhookFailFastAsk(t *testing.T) { +func TestWebhookFailFastAck(t *testing.T) { wh, cancel := newTestWebHooks(t) defer cancel() @@ -1131,6 +1131,68 @@ func TestWebhookFailFastAsk(t *testing.T) { mcb.AssertExpectations(t) } +func TestWebhookFailFastAckBatch(t *testing.T) { + wh, cancel := newTestWebHooks(t) + defer cancel() + + msgID := fftypes.NewUUID() + r := mux.NewRouter() + server := httptest.NewServer(r) + server.Close() + + sub := &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Namespace: "ns1", + }, + } + sub.Options.TransportOptions()["fastack"] = true + event := &core.EventDelivery{ + EnrichedEvent: core.EnrichedEvent{ + Event: core.Event{ + ID: fftypes.NewUUID(), + }, + Message: &core.Message{ + Header: core.MessageHeader{ + ID: msgID, + Type: core.MessageTypeBroadcast, + }, + }, + }, + Subscription: core.SubscriptionRef{ + ID: sub.ID, + }, + } + + count := 0 + waiter := make(chan struct{}) + mcb := wh.callbacks.handlers["ns1"].(*eventsmocks.Callbacks) + mcb.On("DeliveryResponse", mock.Anything, mock.Anything). + Return(nil). + Run(func(a mock.Arguments) { + count++ + if count == 2 { + close(waiter) + } + }) + + // Drive two deliveries, waiting for them both to ack (noting both will fail) + err := wh.BatchDeliveryRequest(wh.ctx, mock.Anything, sub, []*core.CombinedEventDataDelivery{ + {Event: event, Data: core.DataArray{ + {ID: fftypes.NewUUID(), Value: fftypes.JSONAnyPtr(`"value1"`)}, + {ID: fftypes.NewUUID(), Value: fftypes.JSONAnyPtr(`"value2"`)}, + }}, + {Event: event, Data: core.DataArray{ + {ID: fftypes.NewUUID(), Value: fftypes.JSONAnyPtr(`"value1"`)}, + {ID: fftypes.NewUUID(), Value: fftypes.JSONAnyPtr(`"value2"`)}, + }}, + }) + assert.NoError(t, err) + + <-waiter + + mcb.AssertExpectations(t) +} + func TestDeliveryRequestNilMessage(t *testing.T) { wh, cancel := newTestWebHooks(t) defer cancel() @@ -1210,6 +1272,51 @@ func TestDeliveryRequestReplyToReply(t *testing.T) { mcb.AssertExpectations(t) } +func TestBatchDeliveryRequestReplyToReply(t *testing.T) { + wh, cancel := newTestWebHooks(t) + defer cancel() + + yes := true + sub := &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Namespace: "ns1", + }, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + WithData: &yes, + }, + }, + } + sub.Options.TransportOptions()["reply"] = true + event := &core.EventDelivery{ + EnrichedEvent: core.EnrichedEvent{ + Event: core.Event{ + ID: fftypes.NewUUID(), + }, + Message: &core.Message{ + Header: core.MessageHeader{ + ID: fftypes.NewUUID(), + Type: core.MessageTypeBroadcast, + CID: fftypes.NewUUID(), + }, + }, + }, + Subscription: core.SubscriptionRef{ + ID: sub.ID, + }, + } + + mcb := wh.callbacks.handlers["ns1"].(*eventsmocks.Callbacks) + mcb.On("DeliveryResponse", mock.Anything, mock.MatchedBy(func(response *core.EventDeliveryResponse) bool { + return !response.Rejected // should be accepted as a no-op so we can move on to other events + })) + + err := wh.BatchDeliveryRequest(wh.ctx, mock.Anything, sub, []*core.CombinedEventDataDelivery{{Event: event, Data: nil}}) + assert.NoError(t, err) + + mcb.AssertExpectations(t) +} + func TestNamespaceRestarted(t *testing.T) { wh, cancel := newTestWebHooks(t) defer cancel() @@ -1356,3 +1463,7 @@ func TestRequestWithBodyReplyEndToEndWithBatch(t *testing.T) { mcb.AssertExpectations(t) } + +func TestFirstDataNeverNil(t *testing.T) { + assert.NotNil(t, (&whPayload{}).firstData()) +} From d579f585b77f1bc0b0df7e2412bbda9cbf3223cf Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Fri, 7 Jul 2023 00:38:33 -0400 Subject: [PATCH 09/10] Prevent creation of batch subscription on transport without support Signed-off-by: Peter Broadhurst --- docs/reference/types/subscription.md | 2 +- docs/reference/types/wsstart.md | 2 +- docs/swagger/swagger.yaml | 84 +++++++++++++----- internal/coremsgs/en_error_messages.go | 3 +- internal/coremsgs/en_struct_descriptions.go | 2 +- internal/events/event_dispatcher.go | 12 ++- internal/events/event_dispatcher_test.go | 87 ++++++++++++------- internal/events/event_manager.go | 9 ++ internal/events/event_manager_test.go | 22 +++++ internal/events/subscription_manager.go | 14 ++- internal/events/system/events.go | 4 +- internal/events/system/events_test.go | 3 +- internal/events/webhooks/webhooks.go | 6 +- internal/events/websockets/websockets.go | 3 +- internal/events/websockets/websockets_test.go | 5 +- internal/orchestrator/orchestrator_test.go | 2 + internal/orchestrator/subscriptions.go | 17 +++- internal/orchestrator/subscriptions_test.go | 84 ++++++++++++++++++ mocks/eventmocks/event_manager.go | 28 ++++++ pkg/core/subscription.go | 4 +- pkg/events/plugin.go | 4 +- 21 files changed, 315 insertions(+), 82 deletions(-) diff --git a/docs/reference/types/subscription.md b/docs/reference/types/subscription.md index 641de927cc..0a7717fadc 100644 --- a/docs/reference/types/subscription.md +++ b/docs/reference/types/subscription.md @@ -105,7 +105,7 @@ nav_order: 3 | `firstEvent` | Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest' | `SubOptsFirstEvent` | | `readAhead` | The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts | `uint16` | | `withData` | Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports. | `bool` | -| `batch` | Enable batching. Works in conjunction with readAhead which defines the batchSize. | `bool` | +| `batch` | Events are delivered in batches in an ordered array. The batch size is capped to the readAhead limit. The event payload is always an array even if there is a single event in the batch. Commonly used with Webhooks to allow events to be delivered and acknowledged in batches. | `bool` | | `batchTimeout` | When batching is enabled, the optional timeout to send events even when the batch hasn't filled. | `string` | | `fastack` | Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations | `bool` | | `url` | Webhooks only: HTTP url to invoke. Can be relative if a base URL is set in the webhook plugin config | `string` | diff --git a/docs/reference/types/wsstart.md b/docs/reference/types/wsstart.md index 6690a07a4b..6514404189 100644 --- a/docs/reference/types/wsstart.md +++ b/docs/reference/types/wsstart.md @@ -96,7 +96,7 @@ nav_order: 23 | `firstEvent` | Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest' | `SubOptsFirstEvent` | | `readAhead` | The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts | `uint16` | | `withData` | Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports. | `bool` | -| `batch` | Enable batching. Works in conjunction with readAhead which defines the batchSize. | `bool` | +| `batch` | Events are delivered in batches in an ordered array. The batch size is capped to the readAhead limit. The event payload is always an array even if there is a single event in the batch. Commonly used with Webhooks to allow events to be delivered and acknowledged in batches. | `bool` | | `batchTimeout` | When batching is enabled, the optional timeout to send events even when the batch hasn't filled. | `string` | | `fastack` | Webhooks only: When true the event will be acknowledged before the webhook is invoked, allowing parallel invocations | `bool` | | `url` | Webhooks only: HTTP url to invoke. Can be relative if a base URL is set in the webhook plugin config | `string` | diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml index 2053a2bcae..9c97442471 100644 --- a/docs/swagger/swagger.yaml +++ b/docs/swagger/swagger.yaml @@ -27702,8 +27702,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with - readAhead which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is + a single event in the batch. Commonly used with Webhooks + to allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -27984,8 +27987,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered array. + The batch size is capped to the readAhead limit. The event + payload is always an array even if there is a single event + in the batch. Commonly used with Webhooks to allow events + to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -28247,8 +28253,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is a + single event in the batch. Commonly used with Webhooks to + allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -28525,8 +28534,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered array. + The batch size is capped to the readAhead limit. The event + payload is always an array even if there is a single event + in the batch. Commonly used with Webhooks to allow events + to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -28788,8 +28800,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is a + single event in the batch. Commonly used with Webhooks to + allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -29128,8 +29143,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is a + single event in the batch. Commonly used with Webhooks to + allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -36605,8 +36623,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with - readAhead which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is + a single event in the batch. Commonly used with Webhooks + to allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -36880,8 +36901,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered array. + The batch size is capped to the readAhead limit. The event + payload is always an array even if there is a single event + in the batch. Commonly used with Webhooks to allow events + to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -37143,8 +37167,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is a + single event in the batch. Commonly used with Webhooks to + allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -37414,8 +37441,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered array. + The batch size is capped to the readAhead limit. The event + payload is always an array even if there is a single event + in the batch. Commonly used with Webhooks to allow events + to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -37677,8 +37707,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is a + single event in the batch. Commonly used with Webhooks to + allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout @@ -38003,8 +38036,11 @@ paths: description: Subscription options properties: batch: - description: Enable batching. Works in conjunction with readAhead - which defines the batchSize. + description: Events are delivered in batches in an ordered + array. The batch size is capped to the readAhead limit. + The event payload is always an array even if there is a + single event in the batch. Commonly used with Webhooks to + allow events to be delivered and acknowledged in batches. type: boolean batchTimeout: description: When batching is enabled, the optional timeout diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go index b179fbe736..1996ca0c28 100644 --- a/internal/coremsgs/en_error_messages.go +++ b/internal/coremsgs/en_error_messages.go @@ -296,5 +296,6 @@ var ( MsgUnexpectedInterfaceType = ffe("FF10457", "Unexpected interface type: %T", 500) MsgBlockchainConnectorRESTErrConflict = ffe("FF10458", "Conflict from blockchain connector: %s", 409) MsgTokensRESTErrConflict = ffe("FF10459", "Conflict from tokens service: %s", 409) - MsgBatchWithDataNotSupport = ffe("FF10460", "Provided subscription '%s' enables batching and withData which is not support", 400) + MsgBatchWithDataNotSupported = ffe("FF10460", "Provided subscription '%s' enables batching and withData which is not supported", 400) + MsgBatchDeliveryNotSupported = ffe("FF10461", "Batch delivery not supported by transport '%s'", 400) ) diff --git a/internal/coremsgs/en_struct_descriptions.go b/internal/coremsgs/en_struct_descriptions.go index 199fab5e37..391354faf2 100644 --- a/internal/coremsgs/en_struct_descriptions.go +++ b/internal/coremsgs/en_struct_descriptions.go @@ -540,7 +540,7 @@ var ( SubscriptionCoreOptionsFirstEvent = ffm("SubscriptionCoreOptions.firstEvent", "Whether your application would like to receive events from the 'oldest' event emitted by your FireFly node (from the beginning of time), or the 'newest' event (from now), or a specific event sequence. Default is 'newest'") SubscriptionCoreOptionsReadAhead = ffm("SubscriptionCoreOptions.readAhead", "The number of events to stream ahead to your application, while waiting for confirmation of consumption of those events. At least once delivery semantics are used in FireFly, so if your application crashes/reconnects this is the maximum number of events you would expect to be redelivered after it restarts") SubscriptionCoreOptionsWithData = ffm("SubscriptionCoreOptions.withData", "Whether message events delivered over the subscription, should be packaged with the full data of those messages in-line as part of the event JSON payload. Or if the application should make separate REST calls to download that data. May not be supported on some transports.") - SubscriptionCoreOptionsBatch = ffm("SubscriptionCoreOptions.batch", "Enable batching. Works in conjunction with readAhead which defines the batchSize.") + SubscriptionCoreOptionsBatch = ffm("SubscriptionCoreOptions.batch", "Events are delivered in batches in an ordered array. The batch size is capped to the readAhead limit. The event payload is always an array even if there is a single event in the batch. Commonly used with Webhooks to allow events to be delivered and acknowledged in batches.") SubscriptionCoreOptionsBatchTimeout = ffm("SubscriptionCoreOptions.batchTimeout", "When batching is enabled, the optional timeout to send events even when the batch hasn't filled.") // TokenApproval field descriptions diff --git a/internal/events/event_dispatcher.go b/internal/events/event_dispatcher.go index 8a4578b0a9..a862d2dd57 100644 --- a/internal/events/event_dispatcher.go +++ b/internal/events/event_dispatcher.go @@ -86,8 +86,12 @@ func newEventDispatcher(ctx context.Context, enricher *eventEnricher, ei events. } batchTimeout := defaultBatchTimeout - if sub.definition.Options.BatchTimeout != "" { - batchTimeout = fftypes.ParseToDuration(sub.definition.Options.BatchTimeout) + if sub.definition.Options.BatchTimeout != nil && *sub.definition.Options.BatchTimeout != "" { + batchTimeout = fftypes.ParseToDuration(*sub.definition.Options.BatchTimeout) + } + batch := false + if sub.definition.Options.Batch != nil { + batch = *sub.definition.Options.Batch } ed := &eventDispatcher{ ctx: log.WithLogField(log.WithLogField(ctx, @@ -109,7 +113,7 @@ func newEventDispatcher(ctx context.Context, enricher *eventEnricher, ei events. acksNacks: make(chan ackNack), closed: make(chan struct{}), txHelper: txHelper, - batch: sub.definition.Options.Batch, + batch: batch, batchTimeout: batchTimeout, } @@ -157,7 +161,7 @@ func (ed *eventDispatcher) electAndStart() { l.Debugf("Closed before we became leader") return } - // We're ready to go - not + // We're ready to go ed.elected = true ed.eventPoller.start() diff --git a/internal/events/event_dispatcher_test.go b/internal/events/event_dispatcher_test.go index 7368c6365e..2f592c92db 100644 --- a/internal/events/event_dispatcher_test.go +++ b/internal/events/event_dispatcher_test.go @@ -97,6 +97,40 @@ func TestEventDispatcherStartStop(t *testing.T) { ed.close() } +func TestEventDispatcherStartStopBatched(t *testing.T) { + ten := uint16(10) + oldest := core.SubOptsFirstEventOldest + truthy := true + ed, cancel := newTestEventDispatcher(&subscription{ + dispatcherElection: make(chan bool, 1), + definition: &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{Namespace: "ns1", Name: "sub1"}, + Ephemeral: true, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + ReadAhead: &ten, + FirstEvent: &oldest, + Batch: &truthy, + }, + }, + }, + }) + defer cancel() + mdi := ed.database.(*databasemocks.Plugin) + ge := mdi.On("GetEvents", mock.Anything, mock.Anything, mock.Anything).Return([]*core.Event{}, nil, fmt.Errorf("context closed")) + confirmedElected := make(chan bool) + ge.RunFn = func(a mock.Arguments) { + <-confirmedElected + } + + assert.Equal(t, int(10), ed.readAhead) + ed.start() + confirmedElected <- true + close(confirmedElected) + ed.eventPoller.eventNotifier.newEvents <- 12345 + ed.close() +} + func TestMaxReadAhead(t *testing.T) { config.Set(coreconfig.SubscriptionDefaultsReadAhead, 65537) ed, cancel := newTestEventDispatcher(&subscription{ @@ -932,36 +966,20 @@ func TestEventDeliveryClosed(t *testing.T) { cancel() } -// func TestBatchEventDeliveryClosed(t *testing.T) { - -// sub := &subscription{ -// definition: &core.Subscription{}, -// } -// ed, cancel := newTestEventDispatcher(sub) - -// go ed.deliverBatchedEvents() - -// id1 := fftypes.NewUUID() -// ed.eventDelivery <- &core.EventDelivery{ -// EnrichedEvent: core.EnrichedEvent{ -// Event: core.Event{ -// ID: id1, -// }, -// Message: &core.Message{ -// Header: core.MessageHeader{ -// ID: fftypes.NewUUID(), -// }, -// Data: core.DataRefs{ -// {ID: fftypes.NewUUID()}, -// }, -// }, -// }, -// } -// time.Sleep(1000) -// close(ed.eventDelivery) -// time.Sleep(1000) -// cancel() -// } +func TestBatchEventDeliveryClosed(t *testing.T) { + + sub := &subscription{ + definition: &core.Subscription{}, + } + ed, cancel := newTestEventDispatcher(sub) + defer cancel() + + ed.batchTimeout = 1 * time.Minute + ed.eventDelivery <- &core.EventDelivery{} + close(ed.eventDelivery) + + ed.deliverBatchedEvents() +} func TestAckClosed(t *testing.T) { @@ -1099,6 +1117,7 @@ func TestEventDispatcherWithReply(t *testing.T) { func TestEventDeliveryBatch(t *testing.T) { log.SetLevel("debug") var five = uint16(5) + truthy := true sub := &subscription{ dispatcherElection: make(chan bool, 1), definition: &core.Subscription{ @@ -1106,7 +1125,7 @@ func TestEventDeliveryBatch(t *testing.T) { Options: core.SubscriptionOptions{ SubscriptionCoreOptions: core.SubscriptionCoreOptions{ ReadAhead: &five, - Batch: true, + Batch: &truthy, }, }, }, @@ -1152,6 +1171,8 @@ func TestEventDispatcherBatchReadAhead(t *testing.T) { log.SetLevel("debug") var five = uint16(5) subID := fftypes.NewUUID() + truthy := true + oneSec := "1s" sub := &subscription{ dispatcherElection: make(chan bool, 1), definition: &core.Subscription{ @@ -1159,8 +1180,8 @@ func TestEventDispatcherBatchReadAhead(t *testing.T) { Options: core.SubscriptionOptions{ SubscriptionCoreOptions: core.SubscriptionCoreOptions{ ReadAhead: &five, - Batch: true, - BatchTimeout: "1s", + Batch: &truthy, + BatchTimeout: &oneSec, }, }, }, diff --git a/internal/events/event_manager.go b/internal/events/event_manager.go index a7e2dfc618..001a551c44 100644 --- a/internal/events/event_manager.go +++ b/internal/events/event_manager.go @@ -62,6 +62,7 @@ type EventManager interface { CreateUpdateDurableSubscription(ctx context.Context, subDef *core.Subscription, mustNew bool) (err error) EnrichEvent(ctx context.Context, event *core.Event) (*core.EnrichedEvent, error) QueueBatchRewind(batchID *fftypes.UUID) + GetTransportCapabilities(ctx context.Context, transportName string) (*events.Capabilities, error) Start() error WaitStop() @@ -208,6 +209,14 @@ func (em *eventManager) DeletedSubscriptions() chan<- *fftypes.UUID { return em.subManager.deletedSubscriptions } +func (em *eventManager) GetTransportCapabilities(ctx context.Context, transportName string) (*events.Capabilities, error) { + t, err := em.subManager.getTransport(ctx, transportName) + if err != nil { + return nil, err + } + return t.Capabilities(), nil +} + func (em *eventManager) WaitStop() { em.subManager.close() if em.blobReceiver != nil { diff --git a/internal/events/event_manager_test.go b/internal/events/event_manager_test.go index 3b1f16b5aa..f43429e1d5 100644 --- a/internal/events/event_manager_test.go +++ b/internal/events/event_manager_test.go @@ -603,3 +603,25 @@ func TestGetPlugins(t *testing.T) { assert.ElementsMatch(t, em.GetPlugins(), expectedPlugins) } + +func TestGetTransportCapabilities(t *testing.T) { + em := newTestEventManager(t) + defer em.cleanup(t) + + em.mev.On("Capabilities").Return(&events.Capabilities{BatchDelivery: true}) + + c, err := em.GetTransportCapabilities(context.Background(), "websockets") + assert.NoError(t, err) + assert.NotNil(t, c) + assert.True(t, c.BatchDelivery) + + em.mev.AssertExpectations(t) +} + +func TestGetTransportCapabilitiesUnknown(t *testing.T) { + em := newTestEventManager(t) + defer em.cleanup(t) + + _, err := em.GetTransportCapabilities(context.Background(), "wrong") + assert.Regexp(t, "FF10172", err) +} diff --git a/internal/events/subscription_manager.go b/internal/events/subscription_manager.go index da9be8f573..788e72ec31 100644 --- a/internal/events/subscription_manager.go +++ b/internal/events/subscription_manager.go @@ -249,13 +249,21 @@ func (sm *subscriptionManager) deletedDurableSubscription(id *fftypes.UUID) { } } +func (sm *subscriptionManager) getTransport(ctx context.Context, transportName string) (events.Plugin, error) { + transport, ok := sm.transports[transportName] + if !ok { + return nil, i18n.NewError(ctx, coremsgs.MsgUnknownEventTransportPlugin, transportName) + } + return transport, nil +} + // nolint: gocyclo func (sm *subscriptionManager) parseSubscriptionDef(ctx context.Context, subDef *core.Subscription) (sub *subscription, err error) { filter := subDef.Filter - transport, ok := sm.transports[subDef.Transport] - if !ok { - return nil, i18n.NewError(ctx, coremsgs.MsgUnknownEventTransportPlugin, subDef.Transport) + transport, err := sm.getTransport(ctx, subDef.Transport) + if err != nil { + return nil, err } if subDef.Options.TLSConfigName != "" && sm.namespace.TLSConfigs[subDef.Options.TLSConfigName] != nil { diff --git a/internal/events/system/events.go b/internal/events/system/events.go index 484b577944..354be298d7 100644 --- a/internal/events/system/events.go +++ b/internal/events/system/events.go @@ -23,6 +23,8 @@ import ( "github.com/hyperledger/firefly-common/pkg/config" "github.com/hyperledger/firefly-common/pkg/fftypes" + "github.com/hyperledger/firefly-common/pkg/i18n" + "github.com/hyperledger/firefly/internal/coremsgs" "github.com/hyperledger/firefly/pkg/core" "github.com/hyperledger/firefly/pkg/events" ) @@ -135,7 +137,7 @@ func (se *Events) DeliveryRequest(ctx context.Context, connID string, sub *core. } func (se *Events) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.CombinedEventDataDelivery) error { - return nil + return i18n.NewError(ctx, coremsgs.MsgBatchDeliveryNotSupported, se.Name()) // should never happen } func (se *Events) NamespaceRestarted(ns string, startTime time.Time) { diff --git a/internal/events/system/events_test.go b/internal/events/system/events_test.go index 9364f1255f..69f6c3bfb9 100644 --- a/internal/events/system/events_test.go +++ b/internal/events/system/events_test.go @@ -153,5 +153,6 @@ func TestEventDeliveryBatch(t *testing.T) { }, } - se.BatchDeliveryRequest(se.ctx, "id", sub, []*core.CombinedEventDataDelivery{}) + err := se.BatchDeliveryRequest(se.ctx, "id", sub, []*core.CombinedEventDataDelivery{}) + assert.Regexp(t, "FF10461", err) } diff --git a/internal/events/webhooks/webhooks.go b/internal/events/webhooks/webhooks.go index d2c8185dd2..368dc2be63 100644 --- a/internal/events/webhooks/webhooks.go +++ b/internal/events/webhooks/webhooks.go @@ -87,8 +87,10 @@ func (wh *WebHooks) Init(ctx context.Context, config config.Section) (err error) client := ffresty.NewWithConfig(ctx, *ffrestyConfig) *wh = WebHooks{ - ctx: log.WithLogField(ctx, "webhook", wh.connID), - capabilities: &events.Capabilities{}, + ctx: log.WithLogField(ctx, "webhook", wh.connID), + capabilities: &events.Capabilities{ + BatchDelivery: true, + }, callbacks: callbacks{ handlers: make(map[string]events.Callbacks), }, diff --git a/internal/events/websockets/websockets.go b/internal/events/websockets/websockets.go index b7c210ed70..4388aff166 100644 --- a/internal/events/websockets/websockets.go +++ b/internal/events/websockets/websockets.go @@ -219,5 +219,6 @@ func (ws *WebSockets) GetStatus() *core.WebSocketStatus { } func (ws *WebSockets) BatchDeliveryRequest(ctx context.Context, connID string, sub *core.Subscription, events []*core.CombinedEventDataDelivery) error { - return nil + // We should have rejected creation of the subscription, due to us not supporting this in our capabilities + return i18n.NewError(ctx, coremsgs.MsgBatchDeliveryNotSupported, ws.Name()) } diff --git a/internal/events/websockets/websockets_test.go b/internal/events/websockets/websockets_test.go index 2ef36ca2a0..6947d66e77 100644 --- a/internal/events/websockets/websockets_test.go +++ b/internal/events/websockets/websockets_test.go @@ -806,7 +806,7 @@ func TestNamespaceRestartedFailClose(t *testing.T) { mcb.AssertExpectations(t) } -func TestEventDeliveryBatch(t *testing.T) { +func TestEventDeliveryBatchReturnsUnsupported(t *testing.T) { cbs := &eventsmocks.Callbacks{} ws, _, cancel := newTestWebsockets(t, cbs, nil) defer cancel() @@ -817,5 +817,6 @@ func TestEventDeliveryBatch(t *testing.T) { }, } - ws.BatchDeliveryRequest(ws.ctx, "id", sub, []*core.CombinedEventDataDelivery{}) + err := ws.BatchDeliveryRequest(ws.ctx, "id", sub, []*core.CombinedEventDataDelivery{}) + assert.Regexp(t, "FF10461", err) } diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index f0bb81f09f..34b96a6135 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -54,6 +54,7 @@ import ( "github.com/hyperledger/firefly/mocks/txwritermocks" "github.com/hyperledger/firefly/pkg/core" "github.com/hyperledger/firefly/pkg/database" + "github.com/hyperledger/firefly/pkg/events" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -199,6 +200,7 @@ func newTestOrchestrator() *testOrchestrator { tor.mcm.On("Name").Return("mock-cm").Maybe() tor.mmi.On("Name").Return("mock-mm").Maybe() tor.mmp.On("Name").Return("mock-mp").Maybe() + tor.mem.On("GetTransportCapabilities", mock.Anything, mock.Anything).Return(&events.Capabilities{}, nil).Maybe() tor.mds.On("Init", mock.Anything).Maybe() tor.cmi.On("GetCache", mock.Anything).Return(cache.NewUmanagedCache(tor.ctx, 100, 5*time.Minute), nil).Maybe() return tor diff --git a/internal/orchestrator/subscriptions.go b/internal/orchestrator/subscriptions.go index 08b9dc7574..eea6cbd359 100644 --- a/internal/orchestrator/subscriptions.go +++ b/internal/orchestrator/subscriptions.go @@ -47,6 +47,10 @@ func (or *orchestrator) createUpdateSubscription(ctx context.Context, subDef *co if subDef.Transport == system.SystemEventsTransport { return nil, i18n.NewError(ctx, coremsgs.MsgSystemTransportInternal) } + capabilities, err := or.events.GetTransportCapabilities(ctx, subDef.Transport) + if err != nil { + return nil, err + } if subDef.Options.TLSConfigName != "" { if or.namespace.TLSConfigs[subDef.Options.TLSConfigName] == nil { @@ -57,15 +61,20 @@ func (or *orchestrator) createUpdateSubscription(ctx context.Context, subDef *co subDef.Options.TLSConfig = or.namespace.TLSConfigs[subDef.Options.TLSConfigName] } - if subDef.Options.BatchTimeout != "" { - _, err := fftypes.ParseDurationString(subDef.Options.BatchTimeout, time.Millisecond) + if subDef.Options.BatchTimeout != nil && *subDef.Options.BatchTimeout != "" { + _, err := fftypes.ParseDurationString(*subDef.Options.BatchTimeout, time.Millisecond) if err != nil { return nil, err } } - if subDef.Options.Batch && subDef.Options.WithData != nil && *subDef.Options.WithData { - return nil, i18n.NewError(ctx, coremsgs.MsgBatchWithDataNotSupport, subDef.Name) + if subDef.Options.Batch != nil && *subDef.Options.Batch { + if subDef.Options.WithData != nil && *subDef.Options.WithData { + return nil, i18n.NewError(ctx, coremsgs.MsgBatchWithDataNotSupported, subDef.Name) + } + if !capabilities.BatchDelivery { + return nil, i18n.NewError(ctx, coremsgs.MsgBatchDeliveryNotSupported, subDef.Transport) + } } return subDef, or.events.CreateUpdateDurableSubscription(ctx, subDef, mustNew) diff --git a/internal/orchestrator/subscriptions_test.go b/internal/orchestrator/subscriptions_test.go index ad441db585..fcd665b881 100644 --- a/internal/orchestrator/subscriptions_test.go +++ b/internal/orchestrator/subscriptions_test.go @@ -25,6 +25,7 @@ import ( "github.com/hyperledger/firefly-common/pkg/fftypes" "github.com/hyperledger/firefly/internal/events/system" "github.com/hyperledger/firefly/internal/events/webhooks" + "github.com/hyperledger/firefly/mocks/eventmocks" "github.com/hyperledger/firefly/pkg/core" "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/events" @@ -57,6 +58,89 @@ func TestCreateSubscriptionSystemTransport(t *testing.T) { assert.Regexp(t, "FF10266", err) } +func TestCreateSubscriptionBadBatchTimeout(t *testing.T) { + or := newTestOrchestrator() + defer or.cleanup(t) + + badTimeout := "-abc" + _, err := or.CreateSubscription(or.ctx, &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Name: "sub1", + }, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + BatchTimeout: &badTimeout, + }, + WebhookSubOptions: core.WebhookSubOptions{ + URL: "http://example.com", + }, + }, + Transport: "webhooks", + }) + assert.Regexp(t, "FF00137", err) +} + +func TestCreateSubscriptionBatchNotSupported(t *testing.T) { + or := newTestOrchestrator() + defer or.cleanup(t) + + truthy := true + _, err := or.CreateSubscription(or.ctx, &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Name: "sub1", + }, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + Batch: &truthy, + }, + WebhookSubOptions: core.WebhookSubOptions{ + URL: "http://example.com", + }, + }, + Transport: "webhooks", + }) + assert.Regexp(t, "FF10461", err) +} + +func TestCreateSubscriptionBatchWithData(t *testing.T) { + or := newTestOrchestrator() + defer or.cleanup(t) + + truthy := true + _, err := or.CreateSubscription(or.ctx, &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Name: "sub1", + }, + Options: core.SubscriptionOptions{ + SubscriptionCoreOptions: core.SubscriptionCoreOptions{ + WithData: &truthy, + Batch: &truthy, + }, + WebhookSubOptions: core.WebhookSubOptions{ + URL: "http://example.com", + }, + }, + Transport: "webhooks", + }) + assert.Regexp(t, "FF10460", err) +} + +func TestCreateSubscriptionBadTransport(t *testing.T) { + or := newTestOrchestrator() + defer or.cleanup(t) + + or.mem = &eventmocks.EventManager{} + or.mem.On("GetTransportCapabilities", mock.Anything, "wrongun").Return(nil, fmt.Errorf("not found")) + or.events = or.mem + _, err := or.CreateSubscription(or.ctx, &core.Subscription{ + SubscriptionRef: core.SubscriptionRef{ + Name: "sub1", + }, + Transport: "wrongun", + }) + assert.Regexp(t, "not found", err) +} + func TestCreateSubscriptionOk(t *testing.T) { or := newTestOrchestrator() defer or.cleanup(t) diff --git a/mocks/eventmocks/event_manager.go b/mocks/eventmocks/event_manager.go index 1ed436b1bf..20dcadfb07 100644 --- a/mocks/eventmocks/event_manager.go +++ b/mocks/eventmocks/event_manager.go @@ -15,6 +15,8 @@ import ( mock "github.com/stretchr/testify/mock" + pkgevents "github.com/hyperledger/firefly/pkg/events" + sharedstorage "github.com/hyperledger/firefly/pkg/sharedstorage" system "github.com/hyperledger/firefly/internal/events/system" @@ -155,6 +157,32 @@ func (_m *EventManager) GetPlugins() []*core.NamespaceStatusPlugin { return r0 } +// GetTransportCapabilities provides a mock function with given fields: ctx, transportName +func (_m *EventManager) GetTransportCapabilities(ctx context.Context, transportName string) (*pkgevents.Capabilities, error) { + ret := _m.Called(ctx, transportName) + + var r0 *pkgevents.Capabilities + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (*pkgevents.Capabilities, error)); ok { + return rf(ctx, transportName) + } + if rf, ok := ret.Get(0).(func(context.Context, string) *pkgevents.Capabilities); ok { + r0 = rf(ctx, transportName) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*pkgevents.Capabilities) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, transportName) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // NewEvents provides a mock function with given fields: func (_m *EventManager) NewEvents() chan<- int64 { ret := _m.Called() diff --git a/pkg/core/subscription.go b/pkg/core/subscription.go index 20135c8e24..7af6fc85e7 100644 --- a/pkg/core/subscription.go +++ b/pkg/core/subscription.go @@ -92,8 +92,8 @@ type SubscriptionCoreOptions struct { FirstEvent *SubOptsFirstEvent `ffstruct:"SubscriptionCoreOptions" json:"firstEvent,omitempty"` ReadAhead *uint16 `ffstruct:"SubscriptionCoreOptions" json:"readAhead,omitempty"` WithData *bool `ffstruct:"SubscriptionCoreOptions" json:"withData,omitempty"` - Batch bool `ffstruct:"SubscriptionCoreOptions" json:"batch,omitempty"` - BatchTimeout string `ffstruct:"SubscriptionCoreOptions" json:"batchTimeout,omitempty"` + Batch *bool `ffstruct:"SubscriptionCoreOptions" json:"batch,omitempty"` + BatchTimeout *string `ffstruct:"SubscriptionCoreOptions" json:"batchTimeout,omitempty"` } // SubscriptionOptions customize the behavior of subscriptions diff --git a/pkg/events/plugin.go b/pkg/events/plugin.go index 386c955274..03098c5d2a 100644 --- a/pkg/events/plugin.go +++ b/pkg/events/plugin.go @@ -87,4 +87,6 @@ type Callbacks interface { DeliveryResponse(connID string, inflight *core.EventDeliveryResponse) } -type Capabilities struct{} +type Capabilities struct { + BatchDelivery bool +} From be50b97d4a40692d49a5bab4314c167b99cc3433 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Fri, 7 Jul 2023 10:41:53 -0400 Subject: [PATCH 10/10] Move setting the default to earlier in the flow, before valdiation Signed-off-by: Peter Broadhurst --- internal/events/event_manager.go | 15 +++--- internal/events/event_manager_test.go | 33 ++++++++++-- internal/orchestrator/orchestrator_test.go | 2 +- internal/orchestrator/subscriptions.go | 3 +- internal/orchestrator/subscriptions_test.go | 2 +- mocks/eventmocks/event_manager.go | 59 ++++++++++++--------- 6 files changed, 73 insertions(+), 41 deletions(-) diff --git a/internal/events/event_manager.go b/internal/events/event_manager.go index 001a551c44..0bf7a02e30 100644 --- a/internal/events/event_manager.go +++ b/internal/events/event_manager.go @@ -62,7 +62,7 @@ type EventManager interface { CreateUpdateDurableSubscription(ctx context.Context, subDef *core.Subscription, mustNew bool) (err error) EnrichEvent(ctx context.Context, event *core.Event) (*core.EnrichedEvent, error) QueueBatchRewind(batchID *fftypes.UUID) - GetTransportCapabilities(ctx context.Context, transportName string) (*events.Capabilities, error) + ResolveTransportAndCapabilities(ctx context.Context, transportName string) (string, *events.Capabilities, error) Start() error WaitStop() @@ -209,12 +209,15 @@ func (em *eventManager) DeletedSubscriptions() chan<- *fftypes.UUID { return em.subManager.deletedSubscriptions } -func (em *eventManager) GetTransportCapabilities(ctx context.Context, transportName string) (*events.Capabilities, error) { +func (em *eventManager) ResolveTransportAndCapabilities(ctx context.Context, transportName string) (string, *events.Capabilities, error) { + if transportName == "" { + transportName = em.defaultTransport + } t, err := em.subManager.getTransport(ctx, transportName) if err != nil { - return nil, err + return "", nil, err } - return t.Capabilities(), nil + return transportName, t.Capabilities(), nil } func (em *eventManager) WaitStop() { @@ -233,10 +236,6 @@ func (em *eventManager) CreateUpdateDurableSubscription(ctx context.Context, sub return i18n.NewError(ctx, coremsgs.MsgInvalidSubscription) } - if subDef.Transport == "" { - subDef.Transport = em.defaultTransport - } - // Check it can be parsed before inserting (the submanager will check again when processing the creation, so we discard the result) if _, err = em.subManager.parseSubscriptionDef(ctx, subDef); err != nil { return err diff --git a/internal/events/event_manager_test.go b/internal/events/event_manager_test.go index f43429e1d5..df18f72768 100644 --- a/internal/events/event_manager_test.go +++ b/internal/events/event_manager_test.go @@ -396,6 +396,7 @@ func TestCreateDurableSubscriptionDupName(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -411,6 +412,7 @@ func TestCreateDurableSubscriptionDefaultSubCannotParse(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -429,6 +431,7 @@ func TestCreateDurableSubscriptionBadFirstEvent(t *testing.T) { defer em.cleanup(t) wrongFirstEvent := core.SubOptsFirstEvent("lobster") sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -450,6 +453,7 @@ func TestCreateDurableSubscriptionNegativeFirstEvent(t *testing.T) { defer em.cleanup(t) wrongFirstEvent := core.SubOptsFirstEvent("-12345") sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -470,6 +474,7 @@ func TestCreateDurableSubscriptionGetHighestSequenceFailure(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -486,6 +491,7 @@ func TestCreateDurableSubscriptionOk(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -509,6 +515,7 @@ func TestUpdateDurableSubscriptionOk(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) sub := &core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), Namespace: "ns1", @@ -517,6 +524,7 @@ func TestUpdateDurableSubscriptionOk(t *testing.T) { } var firstEvent core.SubOptsFirstEvent = "12345" em.mdi.On("GetSubscriptionByName", mock.Anything, "ns1", "sub1").Return(&core.Subscription{ + Transport: "websockets", SubscriptionRef: core.SubscriptionRef{ ID: fftypes.NewUUID(), }, @@ -604,24 +612,41 @@ func TestGetPlugins(t *testing.T) { assert.ElementsMatch(t, em.GetPlugins(), expectedPlugins) } -func TestGetTransportCapabilities(t *testing.T) { +func TestResolveTransportAndCapabilities(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) em.mev.On("Capabilities").Return(&events.Capabilities{BatchDelivery: true}) - c, err := em.GetTransportCapabilities(context.Background(), "websockets") + resolved, c, err := em.ResolveTransportAndCapabilities(context.Background(), "websockets") assert.NoError(t, err) + assert.Equal(t, "websockets", resolved) assert.NotNil(t, c) assert.True(t, c.BatchDelivery) em.mev.AssertExpectations(t) } -func TestGetTransportCapabilitiesUnknown(t *testing.T) { +func TestResolveTransportAndCapabilitiesUnknown(t *testing.T) { em := newTestEventManager(t) defer em.cleanup(t) - _, err := em.GetTransportCapabilities(context.Background(), "wrong") + _, _, err := em.ResolveTransportAndCapabilities(context.Background(), "wrong") assert.Regexp(t, "FF10172", err) } + +func TestResolveTransportAndCapabilitiesDefault(t *testing.T) { + em := newTestEventManager(t) + defer em.cleanup(t) + em.defaultTransport = "websockets" + + em.mev.On("Capabilities").Return(&events.Capabilities{BatchDelivery: true}) + + resolved, c, err := em.ResolveTransportAndCapabilities(context.Background(), "") + assert.NoError(t, err) + assert.Equal(t, "websockets", resolved) + assert.NotNil(t, c) + assert.True(t, c.BatchDelivery) + + em.mev.AssertExpectations(t) +} diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 34b96a6135..4e86368623 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -200,7 +200,7 @@ func newTestOrchestrator() *testOrchestrator { tor.mcm.On("Name").Return("mock-cm").Maybe() tor.mmi.On("Name").Return("mock-mm").Maybe() tor.mmp.On("Name").Return("mock-mp").Maybe() - tor.mem.On("GetTransportCapabilities", mock.Anything, mock.Anything).Return(&events.Capabilities{}, nil).Maybe() + tor.mem.On("ResolveTransportAndCapabilities", mock.Anything, mock.Anything).Return("websockets", &events.Capabilities{}, nil).Maybe() tor.mds.On("Init", mock.Anything).Maybe() tor.cmi.On("GetCache", mock.Anything).Return(cache.NewUmanagedCache(tor.ctx, 100, 5*time.Minute), nil).Maybe() return tor diff --git a/internal/orchestrator/subscriptions.go b/internal/orchestrator/subscriptions.go index eea6cbd359..fcfd934124 100644 --- a/internal/orchestrator/subscriptions.go +++ b/internal/orchestrator/subscriptions.go @@ -47,10 +47,11 @@ func (or *orchestrator) createUpdateSubscription(ctx context.Context, subDef *co if subDef.Transport == system.SystemEventsTransport { return nil, i18n.NewError(ctx, coremsgs.MsgSystemTransportInternal) } - capabilities, err := or.events.GetTransportCapabilities(ctx, subDef.Transport) + resolvedTransport, capabilities, err := or.events.ResolveTransportAndCapabilities(ctx, subDef.Transport) if err != nil { return nil, err } + subDef.Transport = resolvedTransport if subDef.Options.TLSConfigName != "" { if or.namespace.TLSConfigs[subDef.Options.TLSConfigName] == nil { diff --git a/internal/orchestrator/subscriptions_test.go b/internal/orchestrator/subscriptions_test.go index fcd665b881..58326469f5 100644 --- a/internal/orchestrator/subscriptions_test.go +++ b/internal/orchestrator/subscriptions_test.go @@ -130,7 +130,7 @@ func TestCreateSubscriptionBadTransport(t *testing.T) { defer or.cleanup(t) or.mem = &eventmocks.EventManager{} - or.mem.On("GetTransportCapabilities", mock.Anything, "wrongun").Return(nil, fmt.Errorf("not found")) + or.mem.On("ResolveTransportAndCapabilities", mock.Anything, "wrongun").Return("", nil, fmt.Errorf("not found")) or.events = or.mem _, err := or.CreateSubscription(or.ctx, &core.Subscription{ SubscriptionRef: core.SubscriptionRef{ diff --git a/mocks/eventmocks/event_manager.go b/mocks/eventmocks/event_manager.go index 20dcadfb07..34ffb5a573 100644 --- a/mocks/eventmocks/event_manager.go +++ b/mocks/eventmocks/event_manager.go @@ -157,32 +157,6 @@ func (_m *EventManager) GetPlugins() []*core.NamespaceStatusPlugin { return r0 } -// GetTransportCapabilities provides a mock function with given fields: ctx, transportName -func (_m *EventManager) GetTransportCapabilities(ctx context.Context, transportName string) (*pkgevents.Capabilities, error) { - ret := _m.Called(ctx, transportName) - - var r0 *pkgevents.Capabilities - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (*pkgevents.Capabilities, error)); ok { - return rf(ctx, transportName) - } - if rf, ok := ret.Get(0).(func(context.Context, string) *pkgevents.Capabilities); ok { - r0 = rf(ctx, transportName) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*pkgevents.Capabilities) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, transportName) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // NewEvents provides a mock function with given fields: func (_m *EventManager) NewEvents() chan<- int64 { ret := _m.Called() @@ -236,6 +210,39 @@ func (_m *EventManager) QueueBatchRewind(batchID *fftypes.UUID) { _m.Called(batchID) } +// ResolveTransportAndCapabilities provides a mock function with given fields: ctx, transportName +func (_m *EventManager) ResolveTransportAndCapabilities(ctx context.Context, transportName string) (string, *pkgevents.Capabilities, error) { + ret := _m.Called(ctx, transportName) + + var r0 string + var r1 *pkgevents.Capabilities + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, string) (string, *pkgevents.Capabilities, error)); ok { + return rf(ctx, transportName) + } + if rf, ok := ret.Get(0).(func(context.Context, string) string); ok { + r0 = rf(ctx, transportName) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(context.Context, string) *pkgevents.Capabilities); ok { + r1 = rf(ctx, transportName) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*pkgevents.Capabilities) + } + } + + if rf, ok := ret.Get(2).(func(context.Context, string) error); ok { + r2 = rf(ctx, transportName) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + // SharedStorageBatchDownloaded provides a mock function with given fields: ss, payloadRef, data func (_m *EventManager) SharedStorageBatchDownloaded(ss sharedstorage.Plugin, payloadRef string, data []byte) (*fftypes.UUID, error) { ret := _m.Called(ss, payloadRef, data)