Prevent wrong codeflow subscription configurations#5205
Conversation
…gurations Co-authored-by: premun <7013027+premun@users.noreply.github.com>
I've deduplicated the validation logic as requested. Created a shared Changes in commit 092a101:
|
092a101 to
3aeb2dc
Compare
…scription operations Co-authored-by: premun <7013027+premun@users.noreply.github.com>
Created Changes in commit 03f97b0. |
…ptionsAsync Co-authored-by: premun <7013027+premun@users.noreply.github.com>
...ervice/ProductConstructionService.Api/Api/v2020_02_20/Controllers/SubscriptionsController.cs
Show resolved
Hide resolved
Co-authored-by: premun <7013027+premun@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements validation to prevent conflicting VMR codeflow subscriptions and improves the API design by unifying subscription querying methods. The solution adds comprehensive validation to detect and prevent two types of conflicts: backflow conflicts (multiple VMR→repository subscriptions targeting the same repository branch) and forward flow conflicts (multiple repository→VMR subscriptions flowing into the same VMR branch and target directory).
- Server-side and client-side validation through a shared service architecture
- Unified API with enhanced
GetSubscriptionsAsyncmethod accepting additional filtering parameters - Clear error messages and consistent validation behavior across all entry points
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/SubscriptionsController.cs |
Adds server-side validation logic for detecting codeflow subscription conflicts in Create and UpdateSubscription endpoints |
src/Microsoft.DotNet.Darc/DarcLib/IBasicBarClient.cs |
Extends GetSubscriptionsAsync interface with additional filtering parameters for sourceEnabled, sourceDirectory, and targetDirectory |
src/Microsoft.DotNet.Darc/DarcLib/BarApiClient.cs |
Implements the enhanced GetSubscriptionsAsync method with new filtering parameters |
src/Microsoft.DotNet.Darc/Darc/Operations/SubscriptionOperationBase.cs |
New base class containing shared validation logic for subscription operations |
src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs |
Refactored to inherit from SubscriptionOperationBase and includes client-side validation |
src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs |
Refactored to inherit from SubscriptionOperationBase and includes client-side validation |
src/Maestro/Maestro.DataProviders/SqlBarClient.cs |
Updates data provider to support new filtering parameters in GetSubscriptionsAsync |
test/Microsoft.DotNet.Darc.Tests/Operations/GetSubscriptionsOperationTests.cs |
Updates test mocks to accommodate the new GetSubscriptionsAsync signature |
test/Microsoft.DotNet.Darc.Tests/Operations/GetBuildOperationTests.cs |
Updates test mocks to accommodate the new GetSubscriptionsAsync signature |
...ervice/ProductConstructionService.Api/Api/v2020_02_20/Controllers/SubscriptionsController.cs
Show resolved
Hide resolved
src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs
Outdated
Show resolved
Hide resolved
...ervice/ProductConstructionService.Api/Api/v2020_02_20/Controllers/SubscriptionsController.cs
Show resolved
Hide resolved
…flicts call Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com>
Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com>
This PR implements validation to prevent misconfigured VMR codeflow subscriptions that could lead to conflicts and operational issues, while also improving the API design by unifying subscription querying methods.
Problem
Currently, it's possible to create conflicting codeflow subscriptions that can cause problems:
These misconfigurations can lead to race conditions, merge conflicts, and unexpected behavior in the dependency flow system.
Solution
Comprehensive Validation
Added validation with a shared service architecture:
ICodeflowSubscriptionValidationService: Common interface inMaestro.CommonCodeflowSubscriptionValidationService: Single implementation containing all validation logicCreate()andUpdateSubscription()AddSubscriptionOperationandUpdateSubscriptionOperationSubscriptionOperationBaseeliminates duplication between Add/Update operationsValidation Rules
Backflow subscriptions (VMR → repository,
sourceDirectoryset):Forward flow subscriptions (repository → VMR,
targetDirectoryset):API Improvement
Replaced the specific
GetCodeflowSubscriptionsAsyncmethod with a more genericGetSubscriptionsAsyncthat accepts additional optional parameters:This provides a cleaner, more flexible API that supports both general subscription filtering and specific codeflow validation needs without code duplication.
Error Messages
Clear, actionable error messages help users understand and resolve conflicts:
Benefits
Fixes #5204.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.