Skip to content

Commit 9b31437

Browse files
BryanSoltisBryanSoltis
authored
Merge pull request #191 from BryanSoltis/dev
Add test coverage for ID collision fix, upgrade packages, fix nullabl…
2 parents 29b5bf1 + 1ce67e9 commit 9b31437

18 files changed

Lines changed: 345 additions & 56 deletions

‎src/Services/ResourceDelimiterService.cs‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,13 @@ public async Task<ServiceResponse> PostItemAsync(ResourceDelimiter item)
144144
try
145145
{
146146
// Get list of items
147-
var items = (await _repository.GetAllAsync()).ToList();
147+
var itemsEnumerable = await _repository.GetAllAsync();
148+
if (itemsEnumerable == null)
149+
{
150+
serviceResponse.ResponseObject = "Resource Delimiters not found!";
151+
return serviceResponse;
152+
}
153+
var items = itemsEnumerable.ToList();
148154
if (GeneralHelper.IsNotNull(items))
149155
{
150156
// Set the new id
@@ -200,23 +206,40 @@ public async Task<ServiceResponse> PostItemAsync(ResourceDelimiter item)
200206
}
201207
else
202208
{
203-
item.Id = 1;
204-
item.SortOrder = 1;
209+
// Add new item - ID was already set above
210+
item.SortOrder = items.Count + 1;
211+
212+
// Disable other items if this new item is enabled
213+
if (item.Enabled)
214+
{
215+
foreach (var existingItem in items)
216+
{
217+
existingItem.Enabled = false;
218+
}
219+
}
220+
205221
items.Add(item);
206222
}
223+
}
224+
else
225+
{
226+
// Add first item to empty list
227+
item.SortOrder = 1;
228+
items.Add(item);
229+
}
207230

208-
position = 1;
209-
foreach (ResourceDelimiter thisitem in items.OrderBy(x => x.SortOrder).ToList())
210-
{
211-
thisitem.SortOrder = position;
212-
position += 1;
213-
}
214-
215-
// Write items to file
216-
await _repository.SaveAllAsync(items);
217-
serviceResponse.ResponseObject = "Resource Delimiter added/updated!";
218-
serviceResponse.Success = true;
231+
// Normalize sort order
232+
position = 1;
233+
foreach (ResourceDelimiter thisitem in items.OrderBy(x => x.SortOrder).ToList())
234+
{
235+
thisitem.SortOrder = position;
236+
position += 1;
219237
}
238+
239+
// Write items to file
240+
await _repository.SaveAllAsync(items);
241+
serviceResponse.ResponseObject = "Resource Delimiter added/updated!";
242+
serviceResponse.Success = true;
220243
}
221244
else
222245
{

‎tests/AzureNamingTool.UnitTests/AzureNamingTool.UnitTests.csproj‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<TargetFramework>net8.0</TargetFramework>
4+
<TargetFramework>net10.0</TargetFramework>
55
<Nullable>enable</Nullable>
66

77
<IsPackable>false</IsPackable>
88
</PropertyGroup>
99

1010
<ItemGroup>
1111
<PackageReference Include="FluentAssertions" Version="6.12.0" />
12-
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0"/>
12+
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0"/>
1313
<PackageReference Include="Moq" Version="4.20.70" />
14+
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
1415
<PackageReference Include="xunit" Version="2.4.1"/>
1516
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
1617
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

‎tests/AzureNamingTool.UnitTests/Helpers/ValidationHelperTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -532,10 +532,10 @@ public void ValidateGeneratedName_ShouldHandleNullInvalidCharacterFields()
532532
Regx = "^[a-z0-9-]+$",
533533
LengthMin = "3",
534534
LengthMax = "20",
535-
InvalidCharacters = null,
536-
InvalidCharactersStart = null,
537-
InvalidCharactersEnd = null,
538-
InvalidCharactersConsecutive = null
535+
InvalidCharacters = string.Empty,
536+
InvalidCharactersStart = string.Empty,
537+
InvalidCharactersEnd = string.Empty,
538+
InvalidCharactersConsecutive = string.Empty
539539
};
540540
var name = "test-resource";
541541
var delimiter = "-";

‎tests/AzureNamingTool.UnitTests/Repositories/FileSystemStorageProviderTests.cs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ public async Task GetHealthAsync_ShouldIncludeMetadata_WhenFileSystemIsAccessibl
6363

6464
// Assert
6565
health.Metadata.Should().NotBeNull("metadata should be provided");
66+
health.Metadata!.Should().NotBeNull("metadata should be provided");
6667

6768
// If there was a file locking error (e.g., from concurrent test runs), skip the rest
68-
if (health.Metadata.ContainsKey("Error"))
69+
if (health.Metadata!.ContainsKey("Error"))
6970
{
7071
// File locking can occur in test scenarios - this is acceptable
7172
health.Metadata.Should().ContainKey("SettingsPath", "error metadata should include settings path");
@@ -127,9 +128,9 @@ public async Task GetHealthAsync_ShouldReportFileCount_WhenSettingsDirectoryHasF
127128

128129
// Assert
129130
health.Metadata.Should().ContainKey("FileCount");
130-
var fileCount = health.Metadata["FileCount"];
131+
var fileCount = health.Metadata!["FileCount"];
131132
fileCount.Should().BeOfType<int>("file count should be an integer");
132-
((int)fileCount).Should().BeGreaterThanOrEqualTo(0, "file count should not be negative");
133+
((int)fileCount!).Should().BeGreaterThanOrEqualTo(0, "file count should not be negative");
133134
}
134135

135136
/// <summary>
@@ -149,12 +150,12 @@ public async Task GetHealthAsync_ShouldReportMultipleFiles_InProductionEnvironme
149150

150151
// Assert
151152
health.IsHealthy.Should().BeTrue("production environment should be healthy");
152-
var fileCount = (int)health.Metadata["FileCount"];
153+
var fileCount = (int)health.Metadata!["FileCount"]!;
153154
fileCount.Should().BeGreaterThan(0, "settings directory should contain configuration files");
154155

155156
// Validate typical configuration files exist
156157
health.Metadata.Should().ContainKey("LastModified");
157-
health.Metadata["LastModified"].Should().NotBeNull("should report last modification time");
158+
health.Metadata!["LastModified"].Should().NotBeNull("should report last modification time");
158159
}
159160

160161
[Fact]

‎tests/AzureNamingTool.UnitTests/Services/AdminLogServiceTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public async Task GetItemsAsync_ShouldReturnOrderedItems_WhenItemsExist()
3838
var returnedItems = result.ResponseObject as List<AdminLogMessage>;
3939
returnedItems.Should().NotBeNull();
4040
returnedItems!.Should().HaveCount(3);
41-
returnedItems[0].Id.Should().Be(2); // Most recent first
41+
returnedItems![0].Id.Should().Be(2); // Most recent first
4242
}
4343

4444
[Fact]

‎tests/AzureNamingTool.UnitTests/Services/AdminUserServiceTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ public async Task GetItemsAsync_ShouldReturnOrderedAdminUsers()
4141
var returnedUsers = result.ResponseObject as List<AdminUser>;
4242
returnedUsers.Should().NotBeNull();
4343
returnedUsers!.Should().HaveCount(3);
44-
returnedUsers[0].Name.Should().Be("Alice");
45-
returnedUsers[1].Name.Should().Be("Bob");
46-
returnedUsers[2].Name.Should().Be("Charlie");
44+
returnedUsers![0].Name.Should().Be("Alice");
45+
returnedUsers![1].Name.Should().Be("Bob");
46+
returnedUsers![2].Name.Should().Be("Charlie");
4747
}
4848

4949
[Fact]

‎tests/AzureNamingTool.UnitTests/Services/CustomComponentServiceTests.cs‎

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public async Task GetItemsAsync_ShouldReturnOrderedItems_WhenItemsExist()
4545
var returnedItems = result.ResponseObject as List<CustomComponent>;
4646
returnedItems.Should().NotBeNull();
4747
returnedItems!.Should().HaveCount(2);
48-
returnedItems[0].Name.Should().Be("Component1"); // Sorted by SortOrder
48+
returnedItems![0].Name.Should().Be("Component1"); // Sorted by SortOrder
4949
}
5050

5151
[Fact]
@@ -88,7 +88,7 @@ public async Task GetItemsByParentComponentIdAsync_ShouldReturnFilteredItems_Whe
8888
var returnedItems = result.ResponseObject as List<CustomComponent>;
8989
returnedItems.Should().NotBeNull();
9090
returnedItems!.Should().HaveCount(2);
91-
returnedItems.All(x => x.ParentComponent == "location").Should().BeTrue();
91+
returnedItems!.All(x => x.ParentComponent == "location").Should().BeTrue();
9292
}
9393

9494
[Fact]
@@ -121,4 +121,69 @@ public async Task GetItemsByParentComponentIdAsync_ShouldLogError_WhenExceptionO
121121
result.Success.Should().BeFalse();
122122
_mockAdminLogService.Verify(s => s.PostItemAsync(It.Is<AdminLogMessage>(m => m.Title == "ERROR")), Times.Once);
123123
}
124+
125+
[Fact]
126+
public async Task PostItemAsync_ShouldAssignCorrectId_WhenItemsHaveBeenDeleted()
127+
{
128+
// Arrange - Simulate scenario where IDs 1, 2, 3 exist and ID 2 was deleted
129+
var existingItems = new List<CustomComponent>
130+
{
131+
new CustomComponent { Id = 1, Name = "Component1", ShortName = "c1", ParentComponent = "test", SortOrder = 1 },
132+
new CustomComponent { Id = 3, Name = "Component3", ShortName = "c3", ParentComponent = "test", SortOrder = 2 }
133+
};
134+
135+
var newItem = new CustomComponent
136+
{
137+
Id = 0,
138+
Name = "NewComponent",
139+
ShortName = "new",
140+
ParentComponent = "test"
141+
};
142+
143+
List<CustomComponent>? savedItems = null;
144+
_mockRepository.Setup(r => r.GetAllAsync()).ReturnsAsync(existingItems);
145+
_mockRepository.Setup(r => r.SaveAllAsync(It.IsAny<IEnumerable<CustomComponent>>()))
146+
.Callback<IEnumerable<CustomComponent>>(items => savedItems = items.ToList())
147+
.Returns(Task.CompletedTask);
148+
149+
// Act
150+
var result = await _service.PostItemAsync(newItem);
151+
152+
// Assert
153+
result.Success.Should().BeTrue();
154+
savedItems.Should().NotBeNull();
155+
var addedItem = savedItems!.FirstOrDefault(x => x.Name == "NewComponent");
156+
addedItem.Should().NotBeNull();
157+
addedItem!.Id.Should().Be(4, "because ID should be Max(existing IDs) + 1, not Count + 1");
158+
addedItem.Id.Should().NotBe(3, "to avoid collision with existing item");
159+
}
160+
161+
[Fact]
162+
public async Task PostItemAsync_ShouldAssignId1_WhenNoItemsExist()
163+
{
164+
// Arrange
165+
var emptyList = new List<CustomComponent>();
166+
var newItem = new CustomComponent
167+
{
168+
Id = 0,
169+
Name = "FirstComponent",
170+
ShortName = "first",
171+
ParentComponent = "test"
172+
};
173+
174+
List<CustomComponent>? savedItems = null;
175+
_mockRepository.Setup(r => r.GetAllAsync()).ReturnsAsync(emptyList);
176+
_mockRepository.Setup(r => r.SaveAllAsync(It.IsAny<IEnumerable<CustomComponent>>()))
177+
.Callback<IEnumerable<CustomComponent>>(items => savedItems = items.ToList())
178+
.Returns(Task.CompletedTask);
179+
180+
// Act
181+
var result = await _service.PostItemAsync(newItem);
182+
183+
// Assert
184+
result.Success.Should().BeTrue();
185+
savedItems.Should().NotBeNull();
186+
savedItems!.Should().HaveCount(1);
187+
savedItems![0].Id.Should().Be(1);
188+
}
124189
}

‎tests/AzureNamingTool.UnitTests/Services/GeneratedNamesServiceTests.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public async Task GetItemsAsync_ShouldReturnOrderedByCreatedOn_WhenItemsExist()
4040
var returnedItems = result.ResponseObject as List<GeneratedName>;
4141
returnedItems.Should().NotBeNull();
4242
returnedItems!.Should().HaveCount(2);
43-
returnedItems[0].ResourceName.Should().Be("rg-app-prod"); // Most recent first
44-
returnedItems[1].ResourceName.Should().Be("rg-app-dev");
43+
returnedItems![0].ResourceName.Should().Be("rg-app-prod"); // Most recent first
44+
returnedItems![1].ResourceName.Should().Be("rg-app-dev");
4545
}
4646

4747
[Fact]
@@ -110,7 +110,8 @@ public async Task GetItemAsync_ShouldHandleItemNotFound()
110110
var result = await _service.GetItemAsync(999);
111111

112112
// Assert
113-
result.ResponseObject.Should().Be("Generated Name not found!");
113+
result.ResponseObject!.Should().NotBeNull();
114+
result.ResponseObject!.Should().Be("Generated Name not found!");
114115
}
115116

116117
[Fact]

‎tests/AzureNamingTool.UnitTests/Services/PolicyServiceTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public async Task GetPolicyAsync_ShouldReturnSuccess_WhenAllRepositoriesReturnDa
8282

8383
// Assert
8484
result.Success.Should().BeTrue();
85-
result.ResponseObject.Should().NotBeNull();
85+
((object?)result.ResponseObject).Should().NotBeNull();
8686
var policy = result.ResponseObject as PolicyDefinition;
8787
policy.Should().NotBeNull();
8888
}

‎tests/AzureNamingTool.UnitTests/Services/ResourceComponentServiceTests.cs‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public async Task GetItemsAsync_ShouldReturnEnabledItemsOnly_WhenAdminIsFalse()
4646
var returnedItems = result.ResponseObject as List<ResourceComponent>;
4747
returnedItems.Should().NotBeNull();
4848
returnedItems!.Should().HaveCount(2);
49-
returnedItems.All(x => x.Enabled).Should().BeTrue();
49+
returnedItems!.All(x => x.Enabled).Should().BeTrue();
5050
}
5151

5252
[Fact]
@@ -122,4 +122,69 @@ public async Task GetItemsAsync_ShouldLogError_WhenExceptionOccurs()
122122
result.Success.Should().BeFalse();
123123
_mockAdminLogService.Verify(s => s.PostItemAsync(It.Is<AdminLogMessage>(m => m.Title == "ERROR")), Times.Once);
124124
}
125+
126+
[Fact]
127+
public async Task PostItemAsync_ShouldAssignCorrectId_WhenItemsHaveBeenDeleted()
128+
{
129+
// Arrange - Simulate scenario where IDs 1, 2, 3 exist and ID 2 was deleted
130+
var existingItems = new List<ResourceComponent>
131+
{
132+
new ResourceComponent { Id = 1, Name = "Component1", SortOrder = 1, IsCustom = true },
133+
new ResourceComponent { Id = 3, Name = "Component3", SortOrder = 2, IsCustom = true }
134+
};
135+
136+
var newItem = new ResourceComponent
137+
{
138+
Id = 0,
139+
Name = "NewComponent",
140+
DisplayName = "New Component",
141+
IsCustom = true
142+
};
143+
144+
List<ResourceComponent>? savedItems = null;
145+
_mockRepository.Setup(r => r.GetAllAsync()).ReturnsAsync(existingItems);
146+
_mockRepository.Setup(r => r.SaveAllAsync(It.IsAny<IEnumerable<ResourceComponent>>()))
147+
.Callback<IEnumerable<ResourceComponent>>(items => savedItems = items.ToList())
148+
.Returns(Task.CompletedTask);
149+
150+
// Act
151+
var result = await _service.PostItemAsync(newItem);
152+
153+
// Assert
154+
result.Success.Should().BeTrue();
155+
savedItems.Should().NotBeNull();
156+
var addedItem = savedItems!.FirstOrDefault(x => x.Name == "NewComponent");
157+
addedItem.Should().NotBeNull();
158+
addedItem!.Id.Should().Be(4, "because ID should be Max(existing IDs) + 1, not Count + 1");
159+
addedItem.Id.Should().NotBe(3, "to avoid collision with existing item");
160+
}
161+
162+
[Fact]
163+
public async Task PostItemAsync_ShouldAssignId1_WhenNoItemsExist()
164+
{
165+
// Arrange
166+
var emptyList = new List<ResourceComponent>();
167+
var newItem = new ResourceComponent
168+
{
169+
Id = 0,
170+
Name = "FirstComponent",
171+
DisplayName = "First Component",
172+
IsCustom = true
173+
};
174+
175+
List<ResourceComponent>? savedItems = null;
176+
_mockRepository.Setup(r => r.GetAllAsync()).ReturnsAsync(emptyList);
177+
_mockRepository.Setup(r => r.SaveAllAsync(It.IsAny<IEnumerable<ResourceComponent>>()))
178+
.Callback<IEnumerable<ResourceComponent>>(items => savedItems = items.ToList())
179+
.Returns(Task.CompletedTask);
180+
181+
// Act
182+
var result = await _service.PostItemAsync(newItem);
183+
184+
// Assert
185+
result.Success.Should().BeTrue();
186+
savedItems.Should().NotBeNull();
187+
savedItems!.Should().HaveCount(1);
188+
savedItems![0].Id.Should().Be(1);
189+
}
125190
}

0 commit comments

Comments
 (0)