Registered Implementations Diagnostic Entry

[bhazen]

What issue does this PR address?
Stacks on #2022 to add a diagnostic entry for implementations registered for our interfaces

Important: Any code or remarks in your Pull Request are under the following terms:

If You provide us with any comments, bug reports, feedback, enhancements, or modifications proposed or suggested by You for the Software, such Feedback is provided on a non-confidential basis (notwithstanding any notice to the contrary You may include in any accompanying communication), and Licensor shall have the right to use such Feedback at its discretion, including, but not limited to the incorporation of such suggested changes into the Software. You hereby grant Licensor a perpetual, irrevocable, transferable, sublicensable, nonexclusive license under all rights necessary to incorporate and use your Feedback for any purpose, including to make and sell any products and services.

(see our license, section 7)

[josephdecock]

1. How are we going to handle services that can validly have multiple implementations, such as ISecretParser and IExtensionGrantValidator?
2. How do we decide which public interfaces are included in this list?
3. If we add new interfaces in the future, do we have to remember to update this?

[bhazen]

1. How are we going to handle services that can validly have multiple implementations, such as ISecretParser and IExtensionGrantValidator?
2. How do we decide which public interfaces are included in this list?
3. If we add new interfaces in the future, do we have to remember to update this?

@josephdecock to answer those questions:

1. We'll need to update to handle those differently. I will make that change now. We could call GetServices for every interface and have an array of implementations for everything. Do you have thoughts on that vs only having an array when we expect multiple implementations?
2. I started with all the public interfaces for services and stores as a starting point. I'm open go suggestions on what criteria we should use.
3. As it stands now yes. We could look at doing something with reflection in the constructor to build up the list of interfaces if we wanted something more automated. We'd potentially need to add a way to indicate when an interface can have multiple implementations depending on what direction we decide for point number 1.

[josephdecock]

Do you have thoughts on that vs only having an array when we expect multiple implementations?

Always an array seems simplest, and if multiple implementations are registered unexpectedly we would want to know.

I started with all the public interfaces for services and stores as a starting point. I'm open go suggestions on what criteria we should use.

I think most public interfaces that will always be accessible from the diagnostics host should be captured.

• All of the stores should be included. Right now some are missing. I would organize by structure a bit more. You have stores from D.IS.Storage.Stores and D.IS.Stores all in one list (side note, we have a bit of mess in our namespaces, don't we?). Concretely:
  • Add IBackChannelAuthenticationRequestStore and IRefreshTokenStore
  • Organize IAuthorizationParametersMessageStore, IConsentMessageStore, IMessageStore, IServerSideTicketStore, ISigningCredentialStore, and IValidationKeysStore into their own section, since they are in a different nuget package
• Include all of the Validators
• Include all of the ResponseGenerators
• There are quite a few things that are almost never (or at least rarely) replaced, but it would be very interesting in a support case if they were. I guess I would include these:
  • ICancellationTokenProvider
  • ICorsPolicyService
  • IPersistentGrantSerializer
  • IClock
  • IConcurrencyLock (I think there are multiple implementations for different types T)

I know it's a lot. Sorry.

We probably have to leave out the interfaces from the other packages, like D.IS.Configuration and D.IS.EntityFramework.Storage.

We could look at doing something with reflection in the constructor to build up the list of interfaces if we wanted something more automated. We'd potentially need to add a way to indicate when an interface can have multiple implementations depending on what direction we decide for point number 1.

Could we do reflection in a test? I did that in the mappings to make sure that we didn't forget a model property in an entity (or vice versa).

[bhazen]

Do you have thoughts on that vs only having an array when we expect multiple implementations?

Always an array seems simplest, and if multiple implementations are registered unexpectedly we would want to know.

I started with all the public interfaces for services and stores as a starting point. I'm open go suggestions on what criteria we should use.

I think most public interfaces that will always be accessible from the diagnostics host should be captured.

• All of the stores should be included. Right now some are missing. I would organize by structure a bit more. You have stores from D.IS.Storage.Stores and D.IS.Stores all in one list (side note, we have a bit of mess in our namespaces, don't we?). Concretely:

  • Add IBackChannelAuthenticationRequestStore and IRefreshTokenStore
  • Organize IAuthorizationParametersMessageStore, IConsentMessageStore, IMessageStore, IServerSideTicketStore, ISigningCredentialStore, and IValidationKeysStore into their own section, since they are in a different nuget package

• Include all of the Validators

• Include all of the ResponseGenerators

• There are quite a few things that are almost never (or at least rarely) replaced, but it would be very interesting in a support case if they were. I guess I would include these:

  • ICancellationTokenProvider
  • ICorsPolicyService
  • IPersistentGrantSerializer
  • IClock
  • IConcurrencyLock (I think there are multiple implementations for different types T)

I know it's a lot. Sorry.

We probably have to leave out the interfaces from the other packages, like D.IS.Configuration and D.IS.EntityFramework.Storage.

We could look at doing something with reflection in the constructor to build up the list of interfaces if we wanted something more automated. We'd potentially need to add a way to indicate when an interface can have multiple implementations depending on what direction we decide for point number 1.

Could we do reflection in a test? I did that in the mappings to make sure that we didn't forget a model property in an entity (or vice versa).

I've made changes based on these comments and our offline discussion. Per our off-line discussion about the open generic types: I've included those manually for now and added a comment documenting why that choice was made.