Convert more callback handlers to be namespace-specific #874
Conversation
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Add some utilities and avoid unpacking things more than once. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
These handlers are no-ops, but the signature should match other plugins. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Codecov Report
@@ Coverage Diff @@
## main #874 +/- ##
=======================================
Coverage 99.96% 99.96%
=======================================
Files 299 300 +1
Lines 19483 19504 +21
=======================================
+ Hits 19477 19498 +21
Misses 5 5
Partials 1 1
Continue to review full report at Codecov.
|
| err = json.Unmarshal([]byte(msg.Message), &wrapper) | ||
| switch { | ||
| case err != nil: | ||
| err = fmt.Errorf("invalid transmission from peer '%s': %s", msg.Sender, err) |
There was a problem hiding this comment.
Wondering why these aren't translated errors?
There was a problem hiding this comment.
The error is just logged below (this method never returns errors). I could still localize if you think there's value.
|
|
||
| if err == nil { | ||
| if namespace == "" && msg.RequestID != "" { | ||
| namespace, _, _ = core.ParseNamespacedOpID(h.ctx, msg.RequestID) |
There was a problem hiding this comment.
Noting that if the namespace here is empty/invalid, we catch it in the DXEvent callback (hence no need for error checking here)
internal/dataexchange/ffdx/ffdx.go
Outdated
| } | ||
| return fmt.Errorf("unknown namespace on event '%s'", event.EventID()) |
There was a problem hiding this comment.
Another untranslated error
There was a problem hiding this comment.
Pushed a tweak to address this and make it clear it's just logged and not returned
| @@ -224,6 +211,7 @@ func (em *eventManager) privateBlobReceived(dx dataexchange.Plugin, event dataex | |||
| } | |||
| if br.Namespace != em.namespace { | |||
| log.L(em.ctx).Debugf("Ignoring blob from different namespace '%s'", br.Namespace) | |||
| event.Ack() // Still confirm the event | |||
There was a problem hiding this comment.
Are we guaranteed to only pass the event to a single DX Callback handler? ... if not, what is the implication that event.Ack() could be called multiple times?
There was a problem hiding this comment.
... I remember we made changes to the event ack interface to FFDX, but I can't remember if that included an ID in the ack such that a duplicate was ignored. Feel like maybe I should check that
There was a problem hiding this comment.
Yep - all good, the FFDX API was upgraded to ack-by-id:
https://github.com/hyperledger/firefly-dataexchange-https/blob/10b23db379753b83c40da210fb86ceea4226cb42/src/handlers/events.ts#L100-L120
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Part of FIR-12.
Follow-up to #863.
Only the blockchain plugin remains, and will need to be addressed after #865.