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

Revisit architecture and cleanup #217

Closed
mbrandenburger opened this issue Apr 12, 2022 · 1 comment
Closed

Revisit architecture and cleanup #217

mbrandenburger opened this issue Apr 12, 2022 · 1 comment

Comments

@mbrandenburger
Copy link
Member

A few observations regarding the current code base:

  • The central piece of the token SDK is the service registry. We use providers to create and manage service instances.
    The service registry allows to register providers for specific services (e.g., configuration, network, vault, tms, events,...).
    I believe the name service registry is misleading, as it does not register service instances, but rather their corresponding provider implementations.

  • Some providers maintain references to other providers (nested providers). For instance, the TMS provider requires a vault provider. When a TMS instance is created, it internally keeps a reference to the VaultProvider rather than a reference to a vault instance. It seems that this is not necessary and may be just the result of a quickly growing code base. :) Same issue with the use of the selector provider and certification client provider.

  • The audit provider is actually named audit manager. Looks like a naming inconsistency.

I suggest that we revisit the existing services and introduce a consistent naming convention for these services and their providers. Ideally, services can define or re-use dependency interfaces and service instances can be constructed using a proper constructor function or via direct instantiation with public dependencies. Thereby we enable easy unit testing. Service providers are responsible to construct service instances and fetching the corresponding dependencies.

mbwhite pushed a commit to mbwhite/fabric-token-sdk that referenced this issue Oct 19, 2022
- iou sample app

Signed-off-by: Angelo De Caro <[email protected]>
@adecaro
Copy link
Contributor

adecaro commented Nov 29, 2024

@mbrandenburger is this still relevant? If yes, could you push a PR to address it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants