feat: initial plugin api

[TimoGlastra]

Adds an initial (mostly internal for now) plugin API that will be used as part of modularization. The changes from this PR are also needed for multi-tenancy.

I've not made breaking changes yet, so we can already get this merged and start releasing it, and we can do some clean up of the old custom module approach once we're ready to make the next breaking change release.

As this doesn't contain any breaking changes, I think we should merge this before #881 and #898

The biggest change in this PR is to move from a more implicit registration of services and modules, to a more explicit way to declare services and modules.

Instead of declarating a class with @scoped(Lifecycle.ContainerScoped) it now just needs @injectable (which is exported from the src/plugins directory. Then you can define a plugin to register modules and services. There's currently tow types of plugins, where a module plugin extends a default plugin.

A module is declared like below:

import { module } from '@aries-framework/core'

@module()
class MyModule {
  public static register(dependencyManager: DependencyManager) {
    dependencyManager.registerContextScoped(MyApi)

    dependencyManager.registerSingleton(MyService)
    dependencyManager.registerSingleton(MyRepository)
  }
}

For now all module classes declare the module itself, but as discussed previously the current module classes will be renamed to xxxApi, and a module will act as the combination off everything that module adds (so more than just the public api)

It's not mean to be used publicly yet (and will have breaking changes), but you can then register the module on the dependency manager:

dependencyManager.registerModules(MyModule)

This will make sure the services are registered and in case of the module plugin the module is also registered. The reason why we need this for multitenancy is that we're going to create child container for each tenant agent, that share most of the services from the base agent, but will have some differences. The @scoped(Lifecycle.ContainerScoped) won't do the job anymore.

I've exported some of the tsyringe methods and wrote a simple DependencyManager class to abstract away most of the TSyringe functionality so that you don't need to import from that dependench to write plugins, the needed primivities are exported from AFJ itself.

[codecov-commenter]

Codecov Report

Merging #907 (263f790) into main (3c84265) will increase coverage by 0.12%.
The diff coverage is 98.33%.

@@            Coverage Diff             @@
##             main     #907      +/-   ##
==========================================
+ Coverage   87.65%   87.78%   +0.12%     
==========================================
  Files         470      477       +7     
  Lines       11172    11303     +131     
  Branches     1868     1870       +2     
==========================================
+ Hits         9793     9922     +129     
- Misses       1313     1315       +2     
  Partials       66       66              

Impacted Files	Coverage Δ
...ckages/core/src/transport/HttpOutboundTransport.ts	11.62% <0.00%> (ø)
packages/core/src/transport/WsOutboundTransport.ts	11.59% <0.00%> (ø)
packages/core/src/agent/Agent.ts	95.18% <98.00%> (-0.71%)	⬇️
packages/core/src/agent/Dispatcher.ts	83.33% <100.00%> (ø)
packages/core/src/agent/EnvelopeService.ts	100.00% <100.00%> (ø)
packages/core/src/agent/EventEmitter.ts	100.00% <100.00%> (ø)
packages/core/src/agent/MessageReceiver.ts	80.76% <100.00%> (ø)
packages/core/src/agent/MessageSender.ts	84.61% <100.00%> (ø)
packages/core/src/agent/TransportService.ts	100.00% <100.00%> (ø)
packages/core/src/cache/CacheRepository.ts	100.00% <100.00%> (ø)
... and 68 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c84265...263f790. Read the comment docs.

[berendsliedrecht]

Awesome PR! Great setup for the modularisation :) Could of questions, but nothing blocking.

[TimoGlastra]

@blu3beri updated PR based on your feedback. Thanks for the suggestions, I like the appraoch! :) (also updated the PR body examples)

[karimStekelenburg]

As a first comment (full review coming up):

I remember we discussed this in a WG a few months back. To me, the usage of the terms module and plugin are quite confusing.

Modules

The term module is currently being used in AFJ to refer to a file that contains the public API for a certain topic. However, in my mind a module indicates a sub-component that contains tightly coupled code about a certain topic. This would mean that credentials module doesn't refer to the CredentialsModule.ts file, but rather the entire modules/credentials directory. To me this makes more sense, as the parent directory is also called modules. I also remember agreeing on renaming the ...Module.ts files to ...Api.ts. So CredentialsModule.ts would be renamed to CredentialsApi.ts and the term module would be reserved for the entire directory. This could just be me, so any opinions are more than welcome.

Plugins

Furthermore I'd like to make a naming distinction between internal and external code. I haven't reviewed this in depth yet, but it looks like you're using the term plugin for both internal code, as well as external code. To me a plugin is something optional (kind of like an add-on), something you can use to extend the framework's functionality. I assume the term plugin was chosen in order to avoid conflicting with the term module, but this could be resolved by the remarks I made above.

Suggestion

• Rename all ...Module files and classes to ...Api.
• Only use the @plugin decorator for custom / user-defined plugins.
• If needed, use the a @module decorator for internal code.

[karimStekelenburg]

Another one:

Maybe this will become clear if I run through the entire code base, but just as a quick question...

Say I have the following fictional module/plugin (I'll stick to the naming suggested in my previous comment for clarity):

• /modules
• /modules/foo-module
• /modules/foo-module/FooApi.ts
• /modules/foo-module/FooService.ts
• /modules/foo-module/FooRepository.ts

In this scenario I might like to expose FooApi and FooService so other modules can use them, but maybe I'd like FooRepository to only be used from within the module itself and not expose it to other modules. Is there a way to set such restrictions with the current setup? If not, is this something you plan to add later on?

My reasoning for this is two-fold:

1. If a user writes a plugin where they interact directly with one of the repository classes, this could possibly cause problems.
   • I'm especially worried about this because a lot of the heavy lifting happens in the service layer. Let's say a user directly writes a connection record to storage with state complete, this could cause trouble later on.
2. Simply because I think restricting visibility where needed makes the code base more clear and organised.

As a reference here is how DeepKit does this. They explicitly 'export' files that need to be exposed to other modules.

Keen to hear your thoughts!

[TimoGlastra]

Thanks for the insights @karimStekelenburg! I think you're right in addressing the difference between modules and plugins. Renaming them to Api sounds good to me, but I would like to postpone that for now. This PR intentionally doesn't introduce any breaking changes (yet). However we can already take it into account for the names of the decorators.

What I don't share your thoughts with is to not use internal plugins. I think if we're making an API to dynamically register resources we should try to leverage it, instead of creating a second way to do this internally. I kinda like this approach from ACA-Py also, where internal modules are also plugins, it's just part of the core. This will also make it easier to extract internal parts of the codebase over time. I agree plugins are to extend the framework's functionality, but that could also be for internal code. E.g. the update assistant to me is a separate part that could be seen as a plugin. Extracting it later on into a separate package would basically mean moving the code to a separate directory instead of moving it from our internal 'not plugin' way to the plugin way. I don't see the benefit in that.

So I think we need a way to register a plugin (@plugin() or maybe @module()?), but also need a way to register a plugin with public api. Could be something like @apiPlugin (or apiModule()) but not sure how descriptive that is. Otherwise we can take more of the approach as deepkit does where we register the api parts:

// just  an example, syntax t.b.d.
class Module {
  api = [MyApi],
  services = [MyService, MyRepository]
}

Would you think that works better?

Re the export feature of deepkit, although nice that would be quite complex to implement at the moment. If I understand the docs correctly they have separate injection containers per module, and that's how they can make sure you can't access the classes from another module. I don't think that will be easy to do with TSyringe. I do see your point here though. Especially if we are going to extract modules, we should see changes to services as breaking changes currently as those are often used by other modules. I think we should open a separate issue and address this as part of modularization instead of multi-tenancy

[TimoGlastra]

@blu3beri @karimStekelenburg I've updated my initial comment
with how the API looks like now. Please let me know what you think.

[genaris]

I really like this approach, especially after the very latest changes you introduced, which make it simpler an cleaner from a framework user perspective. Although I'm still not sure if it's better to use the term 'plug-in' or 'module'.

I have a question, mainly because I'm not familiar with Tsyringe magic, about the access of services and repositories: once the module/plug-in is registered, would it be possible to do const myService = agent.dependencyManager.resolve(MyService), or will this resolution be limited to the API (which is registered by calling registerContextScoped instead of registerSingleton)?

[TimoGlastra]

Although I'm still not sure if it's better to use the term 'plug-in' or 'module'.

We're having constant discussion about this 😄, if you have opinions please share! @karimStekelenburg is of the opinion that we should only use the term plugin for external code/modules and shouldn't have internal plug-ins. I don't share that opinion, but I'm fine with both. I think we should rename the current module classes to API soon as I do agree that a module is more than it's public api.

I have a question, mainly because I'm not familiar with Tsyringe magic, about the access of services and repositories: once the module/plug-in is registered, would it be possible to do const myService = agent.dependencyManager.resolve(MyService), or will this resolution be limited to the API (which is registered by calling registerContextScoped instead of registerSingleton)?

That would be possible, yes. Currently we don't have any scoping of services and how you can access them (something we should look at). The registerContextScoped vs registerSingleton means just when a new instance will be returned. singletons will be shared between base agent and all tenants. context scoped will be created per tenant/base agent.

#881 makes all services stateless, meaning we can use them as true singletons (by passing around the agent context)

[karimStekelenburg]

Nice work @TimoGlastra, I really like the @module decorator and ...Api file/class rename (mentioned in the example). Also quite fan of the DependencyManager abstraction.

I think if we do the ...Api renames and have a look at scoped injection containers in follow up PRs, this could really be a robust system. This is a great start and is good to go from my side! 🚀 🚀 🚀

[karimStekelenburg]

@TimoGlastra & @genaris, my opinion on using the term plug-in for external code isn't that strong anymore tbh. My main concern was with the confusion of the word module as it referred to both a file and a directory.

I'm quite happy with the current name changes and don't really mind if we call external 'add-ons' modules or plug-ins. Feelings change 🥲

[TimoGlastra]

Ok in that case I propose we merge this PR and mark the feature as EXPERIMENTAL, that means we can make breaking changes to it without bumping a new minor release. If we decide on what is better (plugin or module) we can do a simple rename

[genaris]

@TimoGlastra & @genaris, my opinion on using the term plug-in for external code isn't that strong anymore tbh. My main concern was with the confusion of the word module as it referred to both a file and a directory.

I'm quite happy with the current name changes and don't really mind if we call external 'add-ons' modules or plug-ins. Feelings change 🥲

Yeah I know that feeling very well 😛. BTW I agree with you with the file/directory naming.

My hesitation with this plug-in/module discussion comes mainly from the nature of the 'plug-ins' we can define in AFJ and the way they are registered and interact with other parts of the framework. When I think about 'plug-ins', the first thing it comes to my mind is a piece of code (script or library) that gives a multimedia player the ability of opening a file encoded in a certain format. Different plug-ins can coexist in an installation and have a common API to register and handle input/output data. In AFJ world we can do analogies in different aspects of the framework: a plug-in can be a module that supports a new protocol, or a module that gives support to a new credential format, or a new DID method (among other things).

Does this 'plug-in API' satisfy these cases? Or would it be better to let each customizable module to handle its own plug-in API? I think it fits pretty well for modules that provide support to a new DIDComm protocol, or any side module that want to make use of some framework resources for a particular functionality (such as storing generic records in the wallet). But maybe it's not so evident for the other cases.

And there is also an inevitable dependency between different modules registered with this API (e.g. some modules need services registered by others, such as ConnectionService), which definitely differ so much from the concept of 'plug-in' I'm used to. So, considering current state of the framework and this API, I think they are closer to 'pluggable modules' than to 'plug-ins', and therefore the term 'module' seems to me more appropriate.

[2byrds]

Great discussion. Words matter. I hesitate to comment but if all 'normal' words seem misleading, consider creating your own word that harkens to the meaning.... pluggables? mod-ins? modables? it's fun, forces others away from assumptions that are wrong. Admittedly some devs will hate it.

[berendsliedrecht]

I have a very minor nitpick and it is mainly to improve the register api. Right now, as defined in a comment above, it is like this:

@module()
class MyModule {
  public static register(dependencyManager: DependencyManager) {
    dependencyManager.registerContextScoped(MyApi)

    dependencyManager.registerSingleton(MyService)
    dependencyManager.registerSingleton(MyRepository)
  }
}

While this is quite nice, I think that passing in the dependencyManager in and deciding with method you need, registerContextScoped, registerSingleton or possible more in the future, can be come difficult.

I propose to change the api to this:

type Return = {
  // TODO: these need way better names to make it more userfriendly and understandable
  contextScoped: Token[]
  singleton: Token[]
}

@module()
class MyModule {
  public static register(): Return  {
    return {
      contextScoped: [MyApi],
      singleton: [MyRepository, MyService]
    }
  }
}

This method hides most of the internals of what specific way you want to register it and IMO makes it a lot less complex. Under the hood ofcourse we still register it in the same way with the dependencyManager.register...(). Quite interested to hear other opinions about this way and if its technically even possible.

[TimoGlastra]

I have a very minor nitpick and it is mainly to improve the register api. Right now, as defined in a comment above, it is like this:

@module()
class MyModule {
  public static register(dependencyManager: DependencyManager) {
    dependencyManager.registerContextScoped(MyApi)

    dependencyManager.registerSingleton(MyService)
    dependencyManager.registerSingleton(MyRepository)
  }
}

While this is quite nice, I think that passing in the dependencyManager in and deciding with method you need, registerContextScoped, registerSingleton or possible more in the future, can be come difficult.

I propose to change the api to this:

type Return = {
  // TODO: these need way better names to make it more userfriendly and understandable
  contextScoped: Token[]
  singleton: Token[]
}

@module()
class MyModule {
  public static register(): Return  {
    return {
      contextScoped: [MyApi],
      singleton: [MyRepository, MyService]
    }
  }
}

This method hides most of the internals of what specific way you want to register it and IMO makes it a lot less complex. Under the hood ofcourse we still register it in the same way with the dependencyManager.register...(). Quite interested to hear other opinions about this way and if its technically even possible.

I'm not too sure on this. It loses some of the possiblities of the imperative way to do it. Tokens are not always the class itself, so we would need to tweak it to work with token => value pairs. So it would need to look something like this:

@module()
class MyModule {
  public static register(): Return  {
    return {
      contextScoped: {
         [MyApi]: MyApi,
       }
       singleton: {
         [MyRepository]: MyRepository,
         [MyService]: MyService,
         [InjectionSymbols.SomethingElse]: MySomethingElse
       }
    }
  } 
}

In addition I do think it sometimes is required to check on the dependency manager if something is already registered, but we may be able to work around that.

[TimoGlastra]

I think they are closer to 'pluggable modules' than to 'plug-ins', and therefore the term 'module' seems to me more appropriate.

I like your elaborate response @genaris and think I agree with your reasoning for why we should go with module over plugin. 👍

Great discussion. Words matter. I hesitate to comment but if all 'normal' words seem misleading, consider creating your own word that harkens to the meaning.... pluggables? mod-ins? modables? it's fun, forces others away from assumptions that are wrong. Admittedly some devs will hate it.

That might work, but I'm worried we risk it becoming even more ambiguous. What's a mod-in, and how would we communicate this clearly to users. But it may be more clear than to put our own meaning to the word module...

[berendsliedrecht]

I'm not too sure on this. It loses some of the possiblities of the imperative way to do it. Tokens are not always the class itself, so we would need to tweak it to work with token => value pairs.

Yeah okay that makes sense. Might be worth thinking about it bit later if we notice any complexity issues with it. For now I am perfectly okay with merging this :).

[TimoGlastra]

I have merged the PR, but feel free to leave additional comments on how to better approach this. This unblocks a lot of other work for me so would like to keep moving. But we can make a final pass before releasing 0.3.0 and improve the api