Feature/add azure dev ops#125
Feature/add azure dev ops#125RalfvandenBurg wants to merge 8 commits intoelsa-workflows:release/3.6.0from
Conversation
Greptile SummaryAdds comprehensive Azure DevOps integration module mirroring the existing GitHub integration structure. Implements 15 activities across four categories: Git repositories/branches, pull requests, work items, and builds. Key Changes:
Issues Found:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/modules/devops/Elsa.DevOps.AzureDevOps/Services/AzureDevOpsConnectionFactory.cs | Connection factory with PAT token storage in dictionary key and no connection disposal |
| src/modules/devops/Elsa.DevOps.AzureDevOps/Activities/AzureDevOpsActivity.cs | Base activity class with proper validation in CanExecuteAsync |
| src/modules/devops/Elsa.DevOps.AzureDevOps/Activities/PullRequests/CreatePullRequest.cs | Creates pull requests with proper validation and ref normalization |
| src/modules/devops/Elsa.DevOps.AzureDevOps/Activities/WorkItems/UpdateWorkItem.cs | Updates work items, creates connection twice when no changes provided |
| src/modules/devops/Elsa.DevOps.AzureDevOps/Features/AzureDevOpsFeature.cs | Feature registration that adds activities and registers connection factory |
Class Diagram
%%{init: {'theme': 'neutral'}}%%
classDiagram
class AzureDevOpsActivity {
+Input~string~ OrganizationUrl
+Input~string~ Token
#CanExecuteAsync() ValueTask~bool~
#GetConnection() VssConnection
}
class AzureDevOpsConnectionFactory {
-Dictionary~string,VssConnection~ _connections
-SemaphoreSlim _semaphore
+GetConnection(string organizationUrl, string token) VssConnection
}
class AzureDevOpsFeature {
+Apply() void
}
class CreatePullRequest {
+Input~string~ Project
+Input~string~ RepositoryName
+Input~string~ SourceBranch
+Input~string~ TargetBranch
+Output~GitPullRequest~ CreatedPullRequest
}
class CreateWorkItem {
+Input~string~ Project
+Input~string~ WorkItemType
+Input~string~ Title
+Output~WorkItem~ CreatedWorkItem
}
class QueueBuild {
+Input~string~ Project
+Input~int~ DefinitionId
+Output~Build~ QueuedBuild
}
class GetBranch {
+Input~string~ Project
+Input~string~ RepositoryName
+Input~string~ BranchName
+Output~GitBranchStats~ Branch
}
AzureDevOpsActivity <|-- CreatePullRequest
AzureDevOpsActivity <|-- CreateWorkItem
AzureDevOpsActivity <|-- QueueBuild
AzureDevOpsActivity <|-- GetBranch
AzureDevOpsActivity ..> AzureDevOpsConnectionFactory : uses
AzureDevOpsFeature ..> AzureDevOpsConnectionFactory : registers
AzureDevOpsFeature ..> AzureDevOpsActivity : discovers activities
Last reviewed commit: 2bc247e
src/modules/devops/Elsa.DevOps.AzureDevOps/Services/AzureDevOpsConnectionFactory.cs
Outdated
Show resolved
Hide resolved
| public class AzureDevOpsConnectionFactory | ||
| { | ||
| private readonly SemaphoreSlim _semaphore = new(1, 1); | ||
| private readonly Dictionary<string, VssConnection> _connections = new(); |
There was a problem hiding this comment.
Connections stored indefinitely without disposal mechanism. VssConnection implements IDisposable but cached instances are never disposed, causing resource leaks.
There was a problem hiding this comment.
Can you validate the change?
There was a problem hiding this comment.
I can see the original code from the diff context you provided, but I'm unable to fetch the current version to validate if changes were made. The PR is quite large (the diff is over 80k characters).
From the diff snippet shown, the issue remains: VssConnection objects are stored in _connections dictionary without any disposal mechanism. Since VssConnection implements IDisposable, these should be disposed when no longer needed.
Could you share the updated version of this file, or let me know what specific changes were made to address the disposal concern?
| var connection = GetConnection(context); | ||
| var witClient = connection.GetClient<Microsoft.TeamFoundation.WorkItemTracking.WebApi.WorkItemTrackingHttpClient>(); | ||
| var existing = await witClient.GetWorkItemAsync(workItemId, null, null, null, context.CancellationToken); | ||
| context.Set(UpdatedWorkItem, existing); | ||
| return; | ||
| } | ||
| var conn = GetConnection(context); | ||
| var client = conn.GetClient<Microsoft.TeamFoundation.WorkItemTracking.WebApi.WorkItemTrackingHttpClient>(); |
There was a problem hiding this comment.
Connection created twice when no fields are updated. Simplify by creating connection once before the if statement.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Can you validate the change?
There was a problem hiding this comment.
Looking at the code, the issue is still present. When document.Count == 0 (lines 80-87), the code creates a connection and client to fetch the existing work item. Then after that block returns, the code creates the connection and client again (lines 88-89) - though these lines won't execute due to the early return.
The problem is the duplication. Here's the fix:
| var connection = GetConnection(context); | |
| var witClient = connection.GetClient<Microsoft.TeamFoundation.WorkItemTracking.WebApi.WorkItemTrackingHttpClient>(); | |
| var existing = await witClient.GetWorkItemAsync(workItemId, null, null, null, context.CancellationToken); | |
| context.Set(UpdatedWorkItem, existing); | |
| return; | |
| } | |
| var conn = GetConnection(context); | |
| var client = conn.GetClient<Microsoft.TeamFoundation.WorkItemTracking.WebApi.WorkItemTrackingHttpClient>(); | |
| var conn = GetConnection(context); | |
| var client = conn.GetClient<Microsoft.TeamFoundation.WorkItemTracking.WebApi.WorkItemTrackingHttpClient>(); | |
| if (document.Count == 0) | |
| { | |
| var existing = await client.GetWorkItemAsync(workItemId, null, null, null, context.CancellationToken); | |
| context.Set(UpdatedWorkItem, existing); | |
| return; | |
| } |
This creates the connection once and reuses the client in both branches.
There was a problem hiding this comment.
The file already has the fix applied from my previous edit — the connection and client are created once on lines 81-82, before the if branch on line 84. Both paths (the early return for no changes, and the update path) share the same connection and client variables. There is no duplicate connection creation.
Are you perhaps looking at a stale version of the file? The current state on disk matches the corrected code.
…sConnectionFactory.cs Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…nBurg/elsa-extensions into feature/AddAzureDevOps
Adds the Elsa.DevOps.AzureDevOps module to integrate Azure DevOps with Elsa Workflows. The implementation mirrors the structure and patterns of Elsa.DevOps.GitHub so workflows can call Azure DevOps Git, pull requests, work items, and builds from activities.
Closes #124