-
Notifications
You must be signed in to change notification settings - Fork 79
Implement configuration ingestion #5636
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
Open
adamzip
wants to merge
25
commits into
dotnet:main
Choose a base branch
from
adamzip:configuration-ingestion-implementation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
2407049
Implement configuration ingestor
adamzip 04c3c99
Update src/Maestro/Maestro.DataProviders/ConfigurationIngestor/Config…
adamzip a5c29f3
Update src/Maestro/Maestro.DataProviders/ISqlBarClient.cs
adamzip 58d7ae3
Update src/Maestro/Maestro.DataProviders/SqlBarClient.cs
adamzip dd64430
Update src/Maestro/Maestro.DataProviders/SqlBarClient.cs
adamzip 32e4cb7
Update src/Microsoft.DotNet.Darc/DarcLib/Models/Yaml/IExternallySynce…
adamzip 27603e0
Update src/Maestro/Maestro.DataProviders/SqlBarClient.cs
adamzip 99ecb06
apply copilot suggestion
adamzip fdf9d75
ignore UniqueId field on yaml models
adamzip 8a91cf5
Fix deletions, rename namespace, add tests
adamzip 7067d8f
Rename method because it's not actually a hash
adamzip f9ca460
Create wrappers for uniqueId member
adamzip 2afadc0
PR improvements
adamzip 800ae0b
Remove wrong usage of IEnumerable
adamzip 43a32e2
Add tests for excluded assets
adamzip 960beaf
More CR changes
adamzip f9f79ef
Merge branch 'main' into configuration-ingestion-implementation
adamzip fea9080
force immutable name, one more test, and fix build
adamzip 1909d6f
CR changes: use namespace include, add target repo ...
adamzip 7934644
Add comment about subscriptions string
adamzip 26e6d3b
Use hash sets instead of dictionaries in method
adamzip b05a405
make not async method not async
adamzip 6fe0036
Change internalsVisibleTo
adamzip 45dbeef
Use IReadOnlyCollection instead of List
adamzip edd6b9e
improve dictionary usages
adamzip File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
14 changes: 14 additions & 0 deletions
14
src/Maestro/Maestro.DataProviders/ConfigurationIngestion/ConfigurationData.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Maestro.DataProviders.ConfigurationIngestion.Helpers; | ||
| using System.Collections.Generic; | ||
|
|
||
| #nullable enable | ||
| namespace Maestro.DataProviders.ConfigurationIngestion; | ||
|
|
||
| public record ConfigurationData( | ||
| IReadOnlyCollection<IngestedSubscription> Subscriptions, | ||
| IReadOnlyCollection<IngestedChannel> Channels, | ||
| IReadOnlyCollection<IngestedDefaultChannel> DefaultChannels, | ||
| IReadOnlyCollection<IngestedBranchMergePolicies> BranchMergePolicies); |
200 changes: 200 additions & 0 deletions
200
src/Maestro/Maestro.DataProviders/ConfigurationIngestion/ConfigurationDataHelper.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using Maestro.Data.Models; | ||
| using Maestro.DataProviders.ConfigurationIngestion.Helpers; | ||
| using Microsoft.DotNet.MaestroConfiguration.Client.Models; | ||
| using Newtonsoft.Json; | ||
| using Newtonsoft.Json.Linq; | ||
|
|
||
| #nullable enable | ||
| namespace Maestro.DataProviders.ConfigurationIngestion; | ||
|
|
||
| internal class ConfigurationDataHelper | ||
| { | ||
| internal static ConfigurationData CreateConfigurationDataObject(Namespace namespaceEntity) | ||
| { | ||
| var convertedSubscriptions = namespaceEntity.Subscriptions | ||
| .Select(sub => SqlBarClient.ToClientModelSubscription(sub)) | ||
| .Select(SubscriptionYaml.FromClientModel) | ||
| .Select(yamlSub => new IngestedSubscription(yamlSub)) | ||
| .ToList(); | ||
|
|
||
| var convertedChannels = namespaceEntity.Channels | ||
| .Select(channel => SqlBarClient.ToClientModelChannel(channel)) | ||
| .Select(ChannelYaml.FromClientModel) | ||
| .Select(yamlChannel => new IngestedChannel(yamlChannel)) | ||
| .ToList(); | ||
|
|
||
| var convertedDefaultChannels = namespaceEntity.DefaultChannels | ||
| .Select(dc => SqlBarClient.ToClientModelDefaultChannel(dc)) | ||
| .Select(DefaultChannelYaml.FromClientModel) | ||
| .Select(yamlDc => new IngestedDefaultChannel(yamlDc)) | ||
| .ToList(); | ||
|
|
||
| var convertedBranchMergePolicies = namespaceEntity.RepositoryBranches | ||
| .Select(rb => SqlBarClient.ToClientModelRepositoryBranch(rb)) | ||
| .Select(BranchMergePoliciesYaml.FromClientModel) | ||
| .Select(rbYaml => new IngestedBranchMergePolicies(rbYaml)) | ||
| .ToList(); | ||
|
|
||
| return new ConfigurationData( | ||
| convertedSubscriptions, | ||
| convertedChannels, | ||
| convertedDefaultChannels, | ||
| convertedBranchMergePolicies); | ||
| } | ||
|
|
||
| internal static ConfigurationDataUpdate ComputeEntityUpdates( | ||
| ConfigurationData configurationData, | ||
| ConfigurationData existingConfigurationData) | ||
| { | ||
| EntityChanges<IngestedSubscription> subscriptionChanges = | ||
| ComputeUpdatesForEntity<IngestedSubscription, Guid>( | ||
| existingConfigurationData.Subscriptions, | ||
| configurationData.Subscriptions); | ||
|
|
||
| EntityChanges<IngestedChannel> channelChanges = | ||
| ComputeUpdatesForEntity<IngestedChannel, string>( | ||
| existingConfigurationData.Channels, | ||
| configurationData.Channels); | ||
|
|
||
| EntityChanges<IngestedDefaultChannel> defaultChannelChanges = | ||
| ComputeUpdatesForEntity<IngestedDefaultChannel, (string, string, string)>( | ||
| existingConfigurationData.DefaultChannels, | ||
| configurationData.DefaultChannels); | ||
|
|
||
| EntityChanges<IngestedBranchMergePolicies> branchMergePolicyChanges = | ||
| ComputeUpdatesForEntity<IngestedBranchMergePolicies, (string, string)>( | ||
| existingConfigurationData.BranchMergePolicies, | ||
| configurationData.BranchMergePolicies); | ||
|
|
||
| return new ConfigurationDataUpdate( | ||
| subscriptionChanges, | ||
| channelChanges, | ||
| defaultChannelChanges, | ||
| branchMergePolicyChanges); | ||
| } | ||
|
|
||
| internal static EntityChanges<T> ComputeUpdatesForEntity<T, TId>( | ||
| IReadOnlyCollection<T> dbEntities, | ||
| IReadOnlyCollection<T> externalEntities) | ||
| where T : class, IExternallySyncedEntity<TId> | ||
| where TId : notnull | ||
| { | ||
| var dbIds = dbEntities.Select(e => e.UniqueId).ToHashSet(); | ||
| var externalIds = externalEntities.Select(e => e.UniqueId).ToHashSet(); | ||
|
|
||
| IReadOnlyCollection<T> creations = [.. externalEntities | ||
| .Where(e => !dbIds.Contains(e.UniqueId))]; | ||
|
|
||
| IReadOnlyCollection<T> removals = [.. dbEntities | ||
| .Where(e => !externalIds.Contains(e.UniqueId))]; | ||
|
|
||
| IReadOnlyCollection<T> updates = [.. externalEntities | ||
| .Where(e => dbIds.Contains(e.UniqueId))]; | ||
|
|
||
| return new EntityChanges<T>(creations, updates, removals); | ||
| } | ||
|
|
||
| internal static Subscription ConvertIngestedSubscriptionToDao( | ||
| IngestedSubscription subscription, | ||
| Namespace namespaceEntity, | ||
| Dictionary<string, Channel> existingChannelsByName) | ||
| { | ||
| if (!existingChannelsByName.TryGetValue(subscription.Values.Channel, out Channel? existingChannel)) | ||
| { | ||
| throw new InvalidOperationException( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. darc exception?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. think we often use those when it's an internal logic exception |
||
| $"Channel '{subscription.Values.Channel}' not found for subscription creation."); | ||
| } | ||
|
|
||
| return new Subscription | ||
| { | ||
| Id = subscription.Values.Id, | ||
| ChannelId = existingChannel.Id, | ||
| Channel = existingChannel, | ||
| SourceRepository = subscription.Values.SourceRepository, | ||
| TargetRepository = subscription.Values.TargetRepository, | ||
| TargetBranch = subscription.Values.TargetBranch, | ||
| PolicyObject = new SubscriptionPolicy | ||
| { | ||
| UpdateFrequency = (UpdateFrequency)(int)subscription.Values.UpdateFrequency, | ||
| Batchable = subscription.Values.Batchable, | ||
| MergePolicies = [.. subscription.Values.MergePolicies.Select(ConvertMergePolicyYamlToDao)], | ||
| }, | ||
| Enabled = subscription.Values.Enabled, | ||
| SourceEnabled = subscription.Values.SourceEnabled, | ||
| SourceDirectory = subscription.Values.SourceDirectory, | ||
| TargetDirectory = subscription.Values.TargetDirectory, | ||
| PullRequestFailureNotificationTags = subscription.Values.FailureNotificationTags, | ||
| ExcludedAssets = subscription.Values.ExcludedAssets == null ? [] : [.. subscription.Values.ExcludedAssets.Select(asset => new AssetFilter() { Filter = asset })], | ||
| Namespace = namespaceEntity, | ||
| }; | ||
| } | ||
|
|
||
| internal static Channel ConvertIngestedChannelToDao( | ||
| IngestedChannel channel, | ||
| Namespace namespaceEntity) | ||
| => new() | ||
| { | ||
| Name = channel.Values.Name, | ||
| Classification = channel.Values.Classification, | ||
| Namespace = namespaceEntity, | ||
| }; | ||
|
|
||
| internal static DefaultChannel ConvertIngestedDefaultChannelToDao( | ||
| IngestedDefaultChannel defaultChannel, | ||
| Namespace namespaceEntity, | ||
| Dictionary<string, Channel> existingChannelsByName) | ||
| { | ||
| if (existingChannelsByName.TryGetValue(defaultChannel.Values.Channel, out Channel? existingChannel)) | ||
| { | ||
| return new DefaultChannel | ||
| { | ||
| ChannelId = existingChannel.Id, | ||
| Channel = existingChannel, | ||
| Repository = defaultChannel.Values.Repository, | ||
| Namespace = namespaceEntity, | ||
| Branch = defaultChannel.Values.Branch, | ||
| Enabled = defaultChannel.Values.Enabled, | ||
| }; | ||
| } | ||
| else | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Channel '{defaultChannel.Values.Channel}' not found for default channel creation."); | ||
| } | ||
| } | ||
|
|
||
| internal static RepositoryBranch ConvertIngestedBranchMergePoliciesToDao( | ||
| IngestedBranchMergePolicies branchMergePolicies, | ||
| Namespace namespaceEntity) | ||
| { | ||
| var policyObject = new RepositoryBranch.Policy | ||
| { | ||
| MergePolicies = [.. branchMergePolicies.Values.MergePolicies.Select(ConvertMergePolicyYamlToDao)], | ||
| }; | ||
|
|
||
| var branchMergePolicyDao = new RepositoryBranch | ||
| { | ||
| RepositoryName = branchMergePolicies.Values.Repository, | ||
| BranchName = branchMergePolicies.Values.Branch, | ||
| PolicyString = JsonConvert.SerializeObject(policyObject), | ||
| Namespace = namespaceEntity, | ||
| }; | ||
|
|
||
| return branchMergePolicyDao; | ||
| } | ||
|
|
||
| private static MergePolicyDefinition ConvertMergePolicyYamlToDao(MergePolicyYaml mergePolicy) | ||
| => new() | ||
| { | ||
| Name = mergePolicy.Name, | ||
| Properties = mergePolicy.Properties?.ToDictionary( | ||
| p => p.Key, | ||
| p => JToken.FromObject(p.Value)), // todo: this seems fragile. Can we change MergePolicyYaml to be <string, JToken> like the DAO & DTO? | ||
| }; | ||
| } | ||
19 changes: 19 additions & 0 deletions
19
src/Maestro/Maestro.DataProviders/ConfigurationIngestion/ConfigurationDataUpdate.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections.Generic; | ||
| using Maestro.DataProviders.ConfigurationIngestion.Helpers; | ||
|
|
||
| #nullable enable | ||
| namespace Maestro.DataProviders.ConfigurationIngestion; | ||
adamzip marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| public record ConfigurationDataUpdate( | ||
| EntityChanges<IngestedSubscription> Subscriptions, | ||
| EntityChanges<IngestedChannel> Channels, | ||
| EntityChanges<IngestedDefaultChannel> DefaultChannels, | ||
| EntityChanges<IngestedBranchMergePolicies> RepositoryBranches); | ||
|
|
||
| public record EntityChanges<T>( | ||
| IReadOnlyCollection<T> Creations, | ||
| IReadOnlyCollection<T> Updates, | ||
| IReadOnlyCollection<T> Removals) where T: class; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this go to the helpers folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
Helpermight not be the best name for this class. It does quite a lot of the core logicThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, all the methods here are internal and static, so it's kind of a helper in that sense (there's zero side effects in that class).
It's hard to find another name, but maybe if i separate the converters into one class and the entity diff computations into another, we can have a converters class and a .. ingestion diff.. computer.. class