Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

E2E suite cleanup #904

Merged
merged 8 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
"mode": "test",
"program": "${fileDirname}",
"env": {
"STACK_FILE": "${env:HOME}/.firefly/stacks/firefly_e2e/stack.json",
"STACK_STATE": "${env:HOME}/.firefly/stacks/firefly_e2e/runtime/stackState.json"
"STACK_DIR": "${env:HOME}/.firefly/stacks/firefly_e2e"
},
"showLog": true
},
Expand Down
5 changes: 0 additions & 5 deletions internal/identity/identitymanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ type identityManager struct {
namespace string
defaultKey string
multipartyRootVerifier *core.VerifierRef
multipartyRootOrg *core.Identity
identityCacheTTL time.Duration
identityCache *ccache.Cache
signingKeyCacheTTL time.Duration
Expand Down Expand Up @@ -303,9 +302,6 @@ func (im *identityManager) FindIdentityForVerifier(ctx context.Context, iTypes [

// GetMultipartyRootOrg returns the identity of the organization that owns the node, if fully registered within the given namespace
func (im *identityManager) GetMultipartyRootOrg(ctx context.Context) (*core.Identity, error) {
if im.multipartyRootOrg != nil {
return im.multipartyRootOrg, nil
}
verifierRef, err := im.GetMultipartyRootVerifier(ctx)
if err != nil {
return nil, err
Expand All @@ -320,7 +316,6 @@ func (im *identityManager) GetMultipartyRootOrg(ctx context.Context) (*core.Iden
if identity.Type != core.IdentityTypeOrg || identity.Name != orgName {
return nil, i18n.NewError(ctx, coremsgs.MsgLocalOrgLookupFailed, orgName, verifierRef.Value)
}
im.multipartyRootOrg = identity
return identity, nil
}

Expand Down
11 changes: 0 additions & 11 deletions internal/identity/identitymanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,17 +738,6 @@ func TestNormalizeKeyViaBlockchainPluginCached(t *testing.T) {

}

func TestGetMultipartyRootOrgCached(t *testing.T) {

ctx, im := newTestIdentityManager(t)
im.multipartyRootOrg = &core.Identity{}

id, err := im.GetMultipartyRootOrg(ctx)
assert.NoError(t, err)
assert.NotNil(t, id)

}

func TestGetMultipartyRootVerifierNotSet(t *testing.T) {

ctx, im := newTestIdentityManager(t)
Expand Down
2 changes: 1 addition & 1 deletion pkg/blockchain/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Plugin interface {
Init(ctx context.Context, config config.Section, metrics metrics.Manager) error

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that it might accidentally deliver events for a different namespace?

Copy link
Member

@shorsher shorsher Jul 19, 2022

Choose a reason for hiding this comment

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

In 1.1, ethconnect/fabconnect subscriptions will be in the format ff-sub-<namespace>-<listener ID>. The plugins will attempt to parse the namespace from the subscription name and then deliver to the correct handler. If not, it will deliver to all plugins.

One scenario where this is likely is existing subscriptions after upgrading to 1.1.

You can see #891 if you're interested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nguyer In short, yes. The managers that register callbacks must therefore always check each event and discard any that are not relevant to them.

Originally there was no filtering at the plugin level (each plugin would deliver all events it received to every subscribed manager). We added namespace-level filtering as a best effort to make things more performant, but as @shorsher mentioned there are situations where it just isn't tenable at the plugin level.

SetHandler(namespace string, handler Callbacks)

// SetOperationHandler registers a handler to receive async operation status
Expand Down
2 changes: 1 addition & 1 deletion pkg/database/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Plugin interface {
Init(ctx context.Context, config config.Section) error

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
SetHandler(namespace string, handler Callbacks)

// Capabilities returns capabilities - not called until after Init
Expand Down
2 changes: 1 addition & 1 deletion pkg/dataexchange/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type Plugin interface {
SetNodes(nodes []fftypes.JSONObject)

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
SetHandler(namespace string, handler Callbacks)

// Data exchange interface must not deliver any events until start is called
Expand Down
2 changes: 1 addition & 1 deletion pkg/identity/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type Plugin interface {
Init(ctx context.Context, config config.Section) error

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
SetHandler(namespace string, handler Callbacks)

// Blockchain interface must not deliver any events until start is called
Expand Down
2 changes: 1 addition & 1 deletion pkg/sharedstorage/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type Plugin interface {
Init(ctx context.Context, config config.Section) error

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
SetHandler(namespace string, handler Callbacks)

// Capabilities returns capabilities - not called until after Init
Expand Down
2 changes: 1 addition & 1 deletion pkg/tokens/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Plugin interface {
Init(ctx context.Context, name string, config config.Section) error

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
SetHandler(namespace string, handler Callbacks) error

// SetOperationHandler registers a handler to receive async operation status
Expand Down
Loading