feat!: agent module registration api #955

TimoGlastra · 2022-07-20T19:53:37Z

Signed-off-by: Timo Glastra timo@animo.id

Add agent module registration api. With this you can now register custom modules on the agent like this:

const agent = new Agent({
  config: { /* agent config */ },
  modules: {
    tenants: new TenantsModule(),
  },
  dependencies: agentDependencies,
})

This will register the tenants module and make the tenants api available. There's some nice typescript magic going on here that automatically infers the api type from the tenants module, allowing the agent to be fully typed with custom modules:

// agent.modules.tenants is fully typed
const tenantAgent = await agent.modules.tenants.getTenantAgent({ tenantId: 'the-tenant-id' })

// tenantAgent is also fully typed, but doesn't have the `agent.api.tenants` available
await tenantAgent.oob.createInvitation()

As you may have already noticed from the example above, all the custom module api's are now nested under the agent.modules property. I've taken this approach for two reasons:

You can define the api for a module using the api key on a module:

class MyModule {
  public api = MyApi

  public register(dependencyManager: DependencyManager) {
    dependencyManager.registerContextScoped(MyApi)
  }
}

All modules that were registered by default on the agent are still registered by default. So agent.oob is present, as well as agent.connections, etc... For now the default modules are still initialized based on the configuration in the agent config, but in a next PR I'll remove that logic and you will only be able to configure modules using the module config itself. If you want to provide custom configuration for a default module you can do so by registering the module. This will just register the module with you custom config, instead of using the default config:

const agent = new Agent({
  config: { /* agent config */ },
  modules: {
    // provide custom config for the default registered connections module.
    connections: new ConnectionsModule({
        autoAcceptConnections: true
    })
  },
  dependencies: agentDependencies,
})

BREAKING CHANGE: custom modules have been moved to the .modules namespace. In addition the agent constructor has been updated to a single options object that contains the config and dependencies properties. Instead of constructing the agent like this:

const agent = new Agent({ /* config */ }, agentDependencies)

You should now construct it like this:

const agent = new Agent({
  config: { /* config */ },
  dependencies: agentDependencies
})

This allows for the new custom modules to be defined in the agent constructor.

codecov-commenter · 2022-07-20T20:22:54Z

Codecov Report

Merging #955 (d6d524f) into 0.3.0-pre (273e353) will increase coverage by 0.03%.
The diff coverage is 99.21%.

@@              Coverage Diff              @@
##           0.3.0-pre     #955      +/-   ##
=============================================
+ Coverage      88.20%   88.23%   +0.03%     
=============================================
  Files            636      637       +1     
  Lines          15036    15092      +56     
  Branches        2536     2542       +6     
=============================================
+ Hits           13262    13316      +54     
- Misses          1676     1678       +2     
  Partials          98       98

Impacted Files	Coverage Δ
packages/core/src/index.ts	`100.00% <ø> (ø)`
packages/core/src/modules/oob/OutOfBandApi.ts	`87.73% <ø> (ø)`
packages/core/src/plugins/Module.ts	`50.00% <ø> (ø)`
packages/core/src/plugins/DependencyManager.ts	`90.32% <90.90%> (-1.35%)`	⬇️
packages/core/src/agent/Agent.ts	`97.34% <100.00%> (-0.26%)`	⬇️
packages/core/src/agent/AgentModules.ts	`100.00% <100.00%> (ø)`
packages/core/src/agent/BaseAgent.ts	`95.40% <100.00%> (+0.05%)`	⬆️
.../src/modules/basic-messages/BasicMessagesModule.ts	`100.00% <100.00%> (ø)`
.../core/src/modules/connections/ConnectionsModule.ts	`100.00% <100.00%> (ø)`
.../core/src/modules/credentials/CredentialsModule.ts	`100.00% <100.00%> (ø)`
... and 14 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

karimStekelenburg · 2022-07-20T20:32:52Z

Nice job @TimoGlastra! My only remark here is that the agent.api naming doesn't make total sense to me (maybe I'm seeing it wrong though). Wouldn't agent.modules make more sense?

TimoGlastra · 2022-07-20T20:37:18Z

My only remark here is that the agent.api naming doesn't make total sense to me (maybe I'm seeing it wrong though). Wouldn't agent.modules make more sense?

Yeah have been thinking about using module, but that also didn't fully make sense to me. It only exposes the api of the module, not the whole module object (so ConnectionsApi class instance, not the ConnectionsModule instance)

karimStekelenburg · 2022-07-20T20:53:33Z

Haha fair, this is a very semantic thingy.

My thought here is this: as a user I will pass modules to the agent constructor. Therefore, I already know that a module adds certain functionality. However, I might not be familiar with the internal structure of modules, so I might not be aware of the concept of FooApi classes.

Since I'm passing modules, I wouldn't be surprised to have to access their APIs through agent.modules.

Anyway, just my thought! I try to think about these things as an outsider, not a framework developer.
This maybe is a good candidate for a WG discussion ;)

TimoGlastra · 2022-07-22T14:30:15Z

Input from WG call:

add custom property that only contains the custom modules. Default modules will still be top level.
use modules instead of api.

I think I agree with the agent.modules part over the agent.api. Not sure I'm a fan of the custom namespace. More feedback is welcome!

berendsliedrecht · 2022-07-22T17:08:48Z

Input from WG call:

add custom property that only contains the custom modules. Default modules will still be top level.

use modules instead of api.

I think I agree with the agent.modules part over the agent.api. Not sure I'm a fan of the custom namespace. More feedback is welcome!

I am slightly in favour of the top-level modules and custom modules defined in agent.modules. I think doing something like agent.modules.credentials.offerCredential becomes so long that it will be annoying to use. Custom modules are IMHO second-class and can be defined under a specific namespace.

Keeping the core-defined modules top level, which in the end might not be a lot, makes a cleaner API.

Having said that, doing agent.oob.XXX For an invitation and then agent.modules.credential.XXX For issuance is very weird.

I am fine with either for different reasons. Maybe we can look at different frameworks and how they solved this issue?

berendsliedrecht · 2022-08-11T18:33:08Z

I thought about this issue a bit more, and it might not be such a big issue to have breaking changes when we add new modules into core. Adding a module into core, after modularisation, is very unlikely to happen when we have the did core modules in there. Unless they expand their spec or we want add seperation, we would not have the add any modules. Of course, it might happen and bundling it with some other breaking changes would be fine IMHO.

Also, as we mentioned in the last WG call, the agent.module.XXX does not really solve the key-collision issue vs the agent.XXX. With this my preference is for agent.XXX unless I am missing something that was another purpose of agent.module.XXX.

TimoGlastra · 2022-08-12T11:22:07Z

Well we need to change the call to Agent.create (or something like that such as a separate method createAgent) instead of calling new Agent(), but otherwise there's no difference.

I'll go with the top level approach then!

genaris · 2022-08-12T11:48:15Z

I don't see any problem on using an agent creator method, especially considering that we currently need to call initialize method after the constructor to make it usable. Could be done anything there to simplify this initialization in cases where applicable?

A question that arises from this approach is if there would be a chance of overriding a core module. We currently do so with connections module to add some project-specific capabilities (we sub-class Agent and define a connections property assigned to a sub-class of ConnectionModule), and even if it feels a bit hacky, it works quite smoothly.

berendsliedrecht · 2022-08-12T12:31:49Z

I don't see any problem on using an agent creator method, especially considering that we currently need to call initialize method after the constructor to make it usable. Could be done anything there to simplify this initialization in cases where applicable?

I think await Agent.create({}).initialize() could also be await Agent.createAndInitialize({}). Seems quite convenient as I think that creating the Agent and initialising it afterwards is something new users might not know.

Also, again not a massive fan of bloating the config, but adding a value to the config with initializeOnCreation: Boolean might also avoid it. However, this might cause issues as Agent.create({}) is synchronous and Agent.initialize is asynchronous and I am not sure if we can handle this properly, return type wise (void | Promise), based on the config field.

A question that arises from this approach is if there would be a chance of overriding a core module. We currently do so with connections module to add some project-specific capabilities (we sub-class Agent and define a connections property assigned to a sub-class of ConnectionModule), and even if it feels a bit hacky, it works quite smoothly.

It would be possible to override a core module.

const agent = Agent.create({
  config: { /* agent config */ },
  modules: {
    CORE_MODULE: new CORE_MODULE({config: 'yes'}), // CORE_MODULE is a core module and here we can define a new one.
  },
  dependencies: agentDependencies,
})

TimoGlastra · 2022-08-12T13:13:54Z

Yes as berend mentioned it would be possible to override the core module by providing it in the agent config, however the typing is currently set to only allow the same module. You would have to write your CustomModule and register it under a different key. We could loosen the type a bit to allow different modules, but I think it makes sense to use a different key if you're not using the connections module. So agent.myConnections or something that contains your business name etc.. would be a bit more explicit maybe? Otherwise we can look at making it accept the same interface (as it extends the connections module)

genaris · 2022-08-16T13:04:15Z

Yes as berend mentioned it would be possible to override the core module by providing it in the agent config, however the typing is currently set to only allow the same module. You would have to write your CustomModule and register it under a different key. We could loosen the type a bit to allow different modules, but I think it makes sense to use a different key if you're not using the connections module. So agent.myConnections or something that contains your business name etc.. would be a bit more explicit maybe? Otherwise we can look at making it accept the same interface (as it extends the connections module)

Certainly, if it can accept the same interface (or an extension of it) it would be great to allow these 'tuned core modules'. Of course there is no problem on using other keys, but if I expose agent.connections and agent.myConnections I'll allow my consumers to use both, and probably that's not something I want.

Anyway, there are some tricks in TypeScript that allow hiding some keys from superclasses, and for sure it's possible to think things differently for using different keys in an elegant way, so it's not a big deal if this idea gets too complicated. Just a 'nice to have' from my point of view.

BREAKING CHANGE: all modules have been moved to the .api namespace. Instead of using agent.xxx (e.g. agent.oob) you should now call agent.api.oob instead. In addition the agent constructor has been updated to a single options object that contains the `config` and `dependencies` properties. Instead of constructing the agent like this: ```ts const agent = new Agent({ /* config */ }, agentDependencies) ``` You should now construct it like this: ```ts const agent = new Agent({ config: { /* config */ }, dependencies: agentDependencies }) ``` This is to alow for the new custom modules that can be befined in the agent constructor. ` Signed-off-by: Timo Glastra <timo@animo.id>

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra · 2022-09-08T18:10:52Z

it's not a big deal if this idea gets too complicated. Just a 'nice to have' from my point of view.

I think it's doable, but maybe for a future PR. This can be added as a non-breaking change, so we could maybe push it to 0.3.1

TimoGlastra · 2022-09-08T18:14:25Z

This PR is now updated and should be ready to merge. I've updated the description of the PR but it boils down to:

default modules are still nested under agent.xxx
custom modules are nested under agent.modules.xxx.

I think this is one of the blockers for the 0.3.0 release, so let's get this merged :)

Signed-off-by: Timo Glastra <timo@animo.id>

genaris

Great! Really love this new module registration API. There are still some doubts to solve in the path of AFJ modularization, but certainly this is an excellent step forward to reach it.

Note: there are a few warnings from GitHub Actions validation

Signed-off-by: Timo Glastra <timo@animo.id>

…gistration

Signed-off-by: Timo Glastra <timo@animo.id>

Signed-off-by: Timo Glastra <timo@animo.id> BREAKING CHANGE: custom modules have been moved to the .modules namespace. In addition the agent constructor has been updated to a single options object that contains the `config` and `dependencies` properties. Instead of constructing the agent like this: ```ts const agent = new Agent({ /* config */ }, agentDependencies) ``` You should now construct it like this: ```ts const agent = new Agent({ config: { /* config */ }, dependencies: agentDependencies }) ``` This allows for the new custom modules to be defined in the agent constructor. Signed-off-by: Ariel Gentile <gentilester@gmail.com>

TimoGlastra added modularization Tasks related to modularization of the framework breaking change PR that introduces a breaking change 0.3.0 Changes for the 0.3.0 release labels Jul 20, 2022

TimoGlastra requested a review from a team as a code owner July 20, 2022 19:53

TimoGlastra added this to the v0.3.0 milestone Jul 25, 2022

TimoGlastra force-pushed the 0.3.0-pre branch 3 times, most recently from f2e61d8 to f38ac05 Compare August 26, 2022 19:09

TimoGlastra added 4 commits August 29, 2022 16:51

test: custom module tests

c1f8564

Signed-off-by: Timo Glastra <timo@animo.id>

refactor: default moduels top level

69483e5

Signed-off-by: Timo Glastra <timo@animo.id>

refactor: api to modules

2b4a740

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra force-pushed the feat/module-registration branch from 4dde7b0 to 2b4a740 Compare September 8, 2022 18:09

style: remove console.log

8ca2735

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra requested a review from genaris September 8, 2022 18:15

genaris approved these changes Sep 8, 2022

View reviewed changes

TimoGlastra added 6 commits September 9, 2022 08:49

Merge branch '0.3.0-pre' into feat/module-registration

84690ca

style: fix eslint warnings

beeb195

Signed-off-by: Timo Glastra <timo@animo.id>

Merge remote-tracking branch 'upstream/0.3.0-pre' into feat/module-re…

2f8aac3

…gistration

style: fix eslint warnings

88dde95

Signed-off-by: Timo Glastra <timo@animo.id>

fix: type issue

eb83b99

Signed-off-by: Timo Glastra <timo@animo.id>

fix: import from src directory

d6d524f

Signed-off-by: Timo Glastra <timo@animo.id>

TimoGlastra merged commit 82a17a3 into openwallet-foundation:0.3.0-pre Sep 10, 2022

TimoGlastra deleted the feat/module-registration branch September 10, 2022 17:59

berendsliedrecht mentioned this pull request Jan 18, 2023

fix/transport connection id animo/aries-framework-javascript#24

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: agent module registration api #955

feat!: agent module registration api #955

TimoGlastra commented Jul 20, 2022 •

edited

Loading

codecov-commenter commented Jul 20, 2022 •

edited

Loading

karimStekelenburg commented Jul 20, 2022

TimoGlastra commented Jul 20, 2022

karimStekelenburg commented Jul 20, 2022 •

edited

Loading

TimoGlastra commented Jul 22, 2022

berendsliedrecht commented Jul 22, 2022

berendsliedrecht commented Aug 11, 2022

TimoGlastra commented Aug 12, 2022 •

edited

Loading

genaris commented Aug 12, 2022

berendsliedrecht commented Aug 12, 2022 •

edited

Loading

TimoGlastra commented Aug 12, 2022

genaris commented Aug 16, 2022

TimoGlastra commented Sep 8, 2022

TimoGlastra commented Sep 8, 2022

genaris left a comment

feat!: agent module registration api #955

feat!: agent module registration api #955

Conversation

TimoGlastra commented Jul 20, 2022 • edited Loading

codecov-commenter commented Jul 20, 2022 • edited Loading

Codecov Report

karimStekelenburg commented Jul 20, 2022

TimoGlastra commented Jul 20, 2022

karimStekelenburg commented Jul 20, 2022 • edited Loading

TimoGlastra commented Jul 22, 2022

berendsliedrecht commented Jul 22, 2022

berendsliedrecht commented Aug 11, 2022

TimoGlastra commented Aug 12, 2022 • edited Loading

genaris commented Aug 12, 2022

berendsliedrecht commented Aug 12, 2022 • edited Loading

TimoGlastra commented Aug 12, 2022

genaris commented Aug 16, 2022

TimoGlastra commented Sep 8, 2022

TimoGlastra commented Sep 8, 2022

genaris left a comment

Choose a reason for hiding this comment

TimoGlastra commented Jul 20, 2022 •

edited

Loading

codecov-commenter commented Jul 20, 2022 •

edited

Loading

karimStekelenburg commented Jul 20, 2022 •

edited

Loading

TimoGlastra commented Aug 12, 2022 •

edited

Loading

berendsliedrecht commented Aug 12, 2022 •

edited

Loading