Skip to content

Commit 65a3ea1

Browse files
Copilotdkurepa
Copilot
and
dkurepa
authored
Make darc delete-channel work with the configuration repo (#5686)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dkurepa <91743470+dkurepa@users.noreply.github.com> Co-authored-by: Djuradj Kurepa <dkurepa@microsoft.com>
1 parent 92a1274 commit 65a3ea1

File tree

5 files changed

+276
-11
lines changed

5 files changed

+276
-11
lines changed

‎src/MaestroConfiguration/src/Microsoft.DotNet.MaestroConfiguration.Client/ConfigurationRepositoryManager.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,17 @@ public async Task AddChannelAsync(ConfigurationRepositoryOperationParameters par
8080
$"Add new channel '{c.Name}'"),
8181
$"Successfully added channel '{channel.Name}' on branch '{parameters.ConfigurationBranch}' of the configuration repository {parameters.RepositoryUri}");
8282

83+
public async Task DeleteChannelAsync(ConfigurationRepositoryOperationParameters parameters, ChannelYaml channel)
84+
=> await PerformConfigurationRepositoryOperationInternal(
85+
parameters,
86+
channel,
87+
(p, repo, branch, c) => DeleteModelInternalAsync(
88+
p, repo, branch, c,
89+
YamlModelUniqueKeys.GetChannelKey,
90+
new ChannelYamlComparer(),
91+
$"Delete channel '{c.Name}'"),
92+
$"Successfully deleted channel '{channel.Name}' from branch '{parameters.ConfigurationBranch}' of the configuration repository {parameters.RepositoryUri}");
93+
8394
public async Task AddDefaultChannelAsync(ConfigurationRepositoryOperationParameters parameters, DefaultChannelYaml defaultChannel)
8495
=> await PerformConfigurationRepositoryOperationInternal(
8596
parameters,

‎src/MaestroConfiguration/src/Microsoft.DotNet.MaestroConfiguration.Client/IConfigurationRepositoryManager.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@ public interface IConfigurationRepositoryManager
1313
Task DeleteSubscriptionAsync(ConfigurationRepositoryOperationParameters parameters, SubscriptionYaml subscription);
1414
Task UpdateSubscriptionAsync(ConfigurationRepositoryOperationParameters parameters, SubscriptionYaml updatedSubscription);
1515
Task AddChannelAsync(ConfigurationRepositoryOperationParameters parameters, ChannelYaml channel);
16+
Task DeleteChannelAsync(ConfigurationRepositoryOperationParameters parameters, ChannelYaml channel);
1617
Task AddDefaultChannelAsync(ConfigurationRepositoryOperationParameters parameters, DefaultChannelYaml defaultChannel);
1718
}

‎src/Microsoft.DotNet.Darc/Darc/Operations/DeleteChannelOperation.cs‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using System.Threading.Tasks;
77
using Microsoft.DotNet.Darc.Options;
88
using Microsoft.DotNet.DarcLib;
9+
using Microsoft.DotNet.MaestroConfiguration.Client;
10+
using Microsoft.DotNet.MaestroConfiguration.Client.Models;
911
using Microsoft.DotNet.ProductConstructionService.Client;
1012
using Microsoft.DotNet.ProductConstructionService.Client.Models;
1113
using Microsoft.Extensions.Logging;
@@ -16,16 +18,19 @@ internal class DeleteChannelOperation : Operation
1618
{
1719
private readonly IBarApiClient _barClient;
1820
private readonly DeleteChannelCommandLineOptions _options;
21+
private readonly IConfigurationRepositoryManager _configurationRepositoryManager;
1922
private readonly ILogger<DeleteChannelOperation> _logger;
2023

2124
public DeleteChannelOperation(
2225
DeleteChannelCommandLineOptions options,
2326
ILogger<DeleteChannelOperation> logger,
24-
IBarApiClient barClient)
27+
IBarApiClient barClient,
28+
IConfigurationRepositoryManager configurationRepositoryManager)
2529
{
2630
_options = options;
2731
_logger = logger;
2832
_barClient = barClient;
33+
_configurationRepositoryManager = configurationRepositoryManager;
2934
}
3035

3136
/// <summary>
@@ -34,19 +39,38 @@ public DeleteChannelOperation(
3439
/// <returns></returns>
3540
public override async Task<int> ExecuteAsync()
3641
{
37-
try
42+
Channel existingChannel = (await _barClient.GetChannelsAsync()).Where(channel => channel.Name.Equals(_options.Name, StringComparison.OrdinalIgnoreCase)).FirstOrDefault();
43+
if (existingChannel == null)
3844
{
39-
// Get the ID of the channel with the specified name.
40-
Channel existingChannel = (await _barClient.GetChannelsAsync()).Where(channel => channel.Name.Equals(_options.Name, StringComparison.OrdinalIgnoreCase)).FirstOrDefault();
45+
_logger.LogError($"Could not find channel with name '{_options.Name}'");
46+
return Constants.ErrorCode;
47+
}
4148

42-
if (existingChannel == null)
49+
try
50+
{
51+
if (_options.ShouldUseConfigurationRepository)
4352
{
44-
_logger.LogError($"Could not find channel with name '{_options.Name}'");
45-
return Constants.ErrorCode;
53+
try
54+
{
55+
await _configurationRepositoryManager.DeleteChannelAsync(
56+
_options.ToConfigurationRepositoryOperationParameters(),
57+
ChannelYaml.FromClientModel(existingChannel));
58+
}
59+
catch (ConfigurationObjectNotFoundException ex)
60+
{
61+
_logger.LogError("No existing channel with name '{name}' found in file {filePath} of repo {repo} on branch {branch}",
62+
existingChannel.Name,
63+
ex.FilePath,
64+
ex.RepositoryUri,
65+
ex.BranchName);
66+
return Constants.ErrorCode;
67+
}
68+
}
69+
else
70+
{
71+
await _barClient.DeleteChannelAsync(existingChannel.Id);
72+
Console.WriteLine($"Successfully deleted channel '{existingChannel.Name}'.");
4673
}
47-
48-
await _barClient.DeleteChannelAsync(existingChannel.Id);
49-
Console.WriteLine($"Successfully deleted channel '{existingChannel.Name}'.");
5074

5175
return Constants.SuccessCode;
5276
}

‎src/Microsoft.DotNet.Darc/Darc/Options/DeleteChannelCommandLineOptions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
namespace Microsoft.DotNet.Darc.Options;
88

99
[Verb("delete-channel", HelpText = "Deletes an existing channel.")]
10-
internal class DeleteChannelCommandLineOptions : CommandLineOptions<DeleteChannelOperation>
10+
internal class DeleteChannelCommandLineOptions : ConfigurationManagementCommandLineOptions<DeleteChannelOperation>
1111
{
1212
[Option('n', "name", Required = true, HelpText = "Name of channel to delete.")]
1313
public string Name { get; set; }

‎test/Microsoft.DotNet.Darc.Tests/Operations/DeleteChannelOperationConfigRepoTests.cs‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
#nullable enable
5+
6+
using System;
7+
using System.Collections.Generic;
8+
using System.IO;
9+
using System.Threading.Tasks;
10+
using AwesomeAssertions;
11+
using Microsoft.DotNet.Darc.Operations;
12+
using Microsoft.DotNet.Darc.Options;
13+
using Microsoft.DotNet.DarcLib.Helpers;
14+
using Microsoft.DotNet.MaestroConfiguration.Client;
15+
using Microsoft.DotNet.MaestroConfiguration.Client.Models;
16+
using Microsoft.DotNet.ProductConstructionService.Client.Models;
17+
using Microsoft.Extensions.Logging;
18+
using Moq;
19+
using NUnit.Framework;
20+
21+
namespace Microsoft.DotNet.Darc.Tests.Operations;
22+
23+
[TestFixture]
24+
public class DeleteChannelOperationConfigRepoTests : ConfigurationManagementTestBase
25+
{
26+
private Mock<ILogger<DeleteChannelOperation>> _loggerMock = null!;
27+
28+
[SetUp]
29+
public override async Task SetupAsync()
30+
{
31+
await base.SetupAsync();
32+
_loggerMock = new Mock<ILogger<DeleteChannelOperation>>();
33+
}
34+
35+
[Test]
36+
public async Task DeleteChannelOperation_WithConfigRepo_RemovesChannelFromFile()
37+
{
38+
// Arrange
39+
var channelToDelete = CreateTestChannel("test-channel-1", "test");
40+
var channelToKeep = CreateTestChannel("test-channel-2", "dev");
41+
var testBranch = GetTestBranch();
42+
43+
// Create channel file with both channels - one to delete and one to keep
44+
var configFilePath = new UnixPath(ConfigFilePathResolver.ChannelFolderPath) / "test-channels.yml";
45+
var existingContent = $"""
46+
{CreateChannelYamlContent(channelToDelete)}
47+
48+
{CreateChannelYamlContent(channelToKeep)}
49+
""";
50+
await CreateFileInConfigRepoAsync(configFilePath.ToString(), existingContent);
51+
52+
SetupGetChannelsAsync([channelToDelete, channelToKeep]);
53+
54+
var options = CreateDeleteChannelOptions(
55+
name: channelToDelete.Name,
56+
configurationBranch: testBranch);
57+
var operation = CreateOperation(options);
58+
59+
// Act
60+
int result = await operation.ExecuteAsync();
61+
62+
// Assert
63+
result.Should().Be(Constants.SuccessCode);
64+
65+
// Verify only the remaining channel is in the file
66+
await CheckoutBranch(testBranch);
67+
var fullPath = Path.Combine(ConfigurationRepoPath, configFilePath.ToString());
68+
File.Exists(fullPath).Should().BeTrue("File should still exist with remaining channel");
69+
70+
var channels = await DeserializeChannelsAsync(fullPath);
71+
channels.Should().HaveCount(1);
72+
channels[0].Name.Should().Be(channelToKeep.Name);
73+
channels[0].Classification.Should().Be(channelToKeep.Classification);
74+
}
75+
76+
[Test]
77+
public async Task DeleteChannelOperation_WithConfigRepo_UsesSpecifiedConfigFilePath()
78+
{
79+
// Arrange - when a specific file path is provided, it should be used directly
80+
var channelToDelete = CreateTestChannel("test-channel-1", "test");
81+
var channelToKeep = CreateTestChannel("test-channel-2", "dev");
82+
var testBranch = GetTestBranch();
83+
84+
// Create channel file at a custom path
85+
var specifiedFilePath = new UnixPath(ConfigFilePathResolver.ChannelFolderPath) / "my-custom-file.yml";
86+
var existingContent = $"""
87+
{CreateChannelYamlContent(channelToDelete)}
88+
89+
{CreateChannelYamlContent(channelToKeep)}
90+
""";
91+
await CreateFileInConfigRepoAsync(specifiedFilePath.ToString(), existingContent);
92+
93+
SetupGetChannelsAsync([channelToDelete, channelToKeep]);
94+
95+
var options = CreateDeleteChannelOptions(
96+
name: channelToDelete.Name,
97+
configurationBranch: testBranch,
98+
configurationFilePath: specifiedFilePath.ToString());
99+
var operation = CreateOperation(options);
100+
101+
// Act
102+
int result = await operation.ExecuteAsync();
103+
104+
// Assert
105+
result.Should().Be(Constants.SuccessCode);
106+
107+
// Verify only the remaining channel is in the file
108+
await CheckoutBranch(testBranch);
109+
var fullPath = Path.Combine(ConfigurationRepoPath, specifiedFilePath.ToString());
110+
File.Exists(fullPath).Should().BeTrue("File should still exist with remaining channel");
111+
112+
var channels = await DeserializeChannelsAsync(fullPath);
113+
channels.Should().HaveCount(1);
114+
channels[0].Name.Should().Be(channelToKeep.Name);
115+
}
116+
117+
[Test]
118+
public async Task DeleteChannelOperation_WithConfigRepo_FindsChannelInNonDefaultFile()
119+
{
120+
// Arrange - channel is in a file that doesn't match the default naming convention
121+
// so the operation must search through all files in the channels folder
122+
var channelToDelete = CreateTestChannel("test-channel-1", "test");
123+
var testBranch = GetTestBranch();
124+
125+
// Create two files that DON'T contain the channel we're looking for
126+
// This ensures the search has to go through multiple files
127+
var unrelatedFile1Path = new UnixPath(ConfigFilePathResolver.ChannelFolderPath) / "aaa-first-file.yml";
128+
var unrelatedChannel1 = CreateTestChannel("unrelated-channel-1", "dev");
129+
await CreateFileInConfigRepoAsync(unrelatedFile1Path.ToString(), CreateChannelYamlContent(unrelatedChannel1));
130+
131+
var unrelatedFile2Path = new UnixPath(ConfigFilePathResolver.ChannelFolderPath) / "bbb-second-file.yml";
132+
var unrelatedChannel2 = CreateTestChannel("unrelated-channel-2", "dev");
133+
await CreateFileInConfigRepoAsync(unrelatedFile2Path.ToString(), CreateChannelYamlContent(unrelatedChannel2));
134+
135+
// Create the file with ONLY the channel we want to delete (file should be deleted after)
136+
var customFilePath = new UnixPath(ConfigFilePathResolver.ChannelFolderPath) / "zzz-custom-channels.yml";
137+
await CreateFileInConfigRepoAsync(customFilePath.ToString(), CreateChannelYamlContent(channelToDelete));
138+
139+
SetupGetChannelsAsync([channelToDelete, unrelatedChannel1, unrelatedChannel2]);
140+
141+
// Note: we do NOT specify configurationFilePath - the operation should find it by searching
142+
var options = CreateDeleteChannelOptions(
143+
name: channelToDelete.Name,
144+
configurationBranch: testBranch);
145+
var operation = CreateOperation(options);
146+
147+
// Act
148+
int result = await operation.ExecuteAsync();
149+
150+
// Assert
151+
result.Should().Be(Constants.SuccessCode);
152+
153+
// Verify the file was deleted since it had only one channel
154+
await CheckoutBranch(testBranch);
155+
var fullPath = Path.Combine(ConfigurationRepoPath, customFilePath.ToString());
156+
File.Exists(fullPath).Should().BeFalse("File should be deleted when last channel is removed");
157+
158+
// Verify the unrelated files were not modified
159+
var unrelatedFile1FullPath = Path.Combine(ConfigurationRepoPath, unrelatedFile1Path.ToString());
160+
var unrelatedFile1Channels = await DeserializeChannelsAsync(unrelatedFile1FullPath);
161+
unrelatedFile1Channels.Should().HaveCount(1);
162+
unrelatedFile1Channels[0].Name.Should().Be(unrelatedChannel1.Name);
163+
164+
var unrelatedFile2FullPath = Path.Combine(ConfigurationRepoPath, unrelatedFile2Path.ToString());
165+
var unrelatedFile2Channels = await DeserializeChannelsAsync(unrelatedFile2FullPath);
166+
unrelatedFile2Channels.Should().HaveCount(1);
167+
unrelatedFile2Channels[0].Name.Should().Be(unrelatedChannel2.Name);
168+
}
169+
170+
#region Helper methods
171+
172+
private Channel CreateTestChannel(string name, string classification)
173+
{
174+
return new Channel(1, name, classification);
175+
}
176+
177+
private static string CreateChannelYamlContent(Channel channel)
178+
{
179+
return $"""
180+
- Name: {channel.Name}
181+
Classification: {channel.Classification}
182+
""";
183+
}
184+
185+
private void SetupGetChannelsAsync(List<Channel> channels)
186+
{
187+
BarClientMock
188+
.Setup(x => x.GetChannelsAsync())
189+
.ReturnsAsync(channels);
190+
}
191+
192+
private DeleteChannelCommandLineOptions CreateDeleteChannelOptions(
193+
string name,
194+
string? configurationBranch = null,
195+
string configurationBaseBranch = DefaultBranch,
196+
string? configurationFilePath = null,
197+
bool noPr = true)
198+
{
199+
return new DeleteChannelCommandLineOptions
200+
{
201+
Name = name,
202+
ConfigurationRepository = ConfigurationRepoPath,
203+
ConfigurationBranch = configurationBranch,
204+
ConfigurationBaseBranch = configurationBaseBranch,
205+
ConfigurationFilePath = configurationFilePath,
206+
NoPr = noPr
207+
};
208+
}
209+
210+
private DeleteChannelOperation CreateOperation(DeleteChannelCommandLineOptions options)
211+
{
212+
return new DeleteChannelOperation(
213+
options,
214+
_loggerMock.Object,
215+
BarClientMock.Object,
216+
ConfigurationRepositoryManager);
217+
}
218+
219+
/// <summary>
220+
/// Deserializes a YAML file containing a list of channels.
221+
/// </summary>
222+
private static async Task<List<ChannelYaml>> DeserializeChannelsAsync(string filePath)
223+
{
224+
var content = await File.ReadAllTextAsync(filePath);
225+
return YamlDeserializer.Deserialize<List<ChannelYaml>>(content) ?? [];
226+
}
227+
228+
#endregion
229+
}

0 commit comments

Comments
 (0)