Skip to content

support task registration from interface with inheritance, generics, method overloading#1021

Merged
cgillum merged 4 commits intoAzure:mainfrom
kevxms:fix-reflection-based-task-activity-name
Jan 12, 2024
Merged

support task registration from interface with inheritance, generics, method overloading#1021
cgillum merged 4 commits intoAzure:mainfrom
kevxms:fix-reflection-based-task-activity-name

Conversation

@kevxms
Copy link
Contributor

@kevxms kevxms commented Dec 29, 2023

there is a bug in TaskHubWorker.AddTaskActivitiesFromInterface where each method of a registered interface is keyed by a weak "qualified name" that hits collisions when using C# code reuse features such as inheritance, generics, method overloading.
This is reproducible even with useFullyQualifiedMethodNames = true.
This PR fixes by making a stronger qualified name in "V2" methods for task registration and client creation.
It is essential to keep the "V1" methods for back-compatibility.

@sainipankaj90k
Copy link

@davidmrdavid adding you for review.

This is for updating interface methods registration as task-activity for objects implementing interfaces with generic type.

Copy link
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the change seems good. I think there's just a few things I'd like to have clarified via additional comments in the code.

@cgillum
Copy link
Member

cgillum commented Jan 12, 2024

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 12, 2024

EDIT: Azure Pipelines run scheduled successfully.

@davidmrdavid
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 12, 2024

EDIT: Azure Pipelines run scheduled successfully.

@cgillum cgillum merged commit 5d3f27a into Azure:main Jan 12, 2024
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.

4 participants