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

Add support for generic services #510

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

APErebus
Copy link
Collaborator

@APErebus APErebus commented Oct 17, 2023

Background

Halibut doesn't currently support generic services (i.e. IMyService<T>). This is due to how the service name is serialized in the request message, it just uses Type.Name, which doesn't include any generic type argument information, just the number of generic type args (i.e. IMyService`1).

This means that the service can't be correctly resolved as IMyService<int> and IMyService<string> are identified in the request message as the same service.

Results

This PR changes the TypeRegistry and the HalibutProxy to use a new extension method, GetGenericQualifiedName() as the service name, rather than the .Name property.

This new extension method adds the generic type args to the name in the format IMyService`2[Arg1,Arg2]. If a generic type arg is also generic, it will build the same structure like this IMyService`2[MyGenericType`1[string],string].

Also, I have updated the TypeRegistry to find all derived types if a service method parameter is abstract. That is, if the method signature is void MyMethod(BaseType base) and there are two derived types of BaseType, then those types are also registered.

I have also added new unit tests covering the new generic services. I don't have enough knowledge of Halibut testing to understand what I need to do regarding the latest/previous version test cases, but this should be backwards compatible for existing services.

How to review this PR

Quality ✔️

Pre-requisites

  • I have read How we use GitHub Issues for help deciding when and where it's appropriate to make an issue.
  • I have considered informing or consulting the right people, according to the ownership map.
  • I have considered appropriate testing for my change.

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

Successfully merging this pull request may close these issues.

None yet

1 participant