Skip to content

Commit d82df51

Browse files
joshsmithxrmclaude
andauthored
feat: add Phase 4 live Dataverse tests and complete Phase 2/3 gaps (#55) (#102)
* feat: add Phase 4 live Dataverse tests and complete Phase 2/3 gaps (#55) Phase 4 - Live Dataverse Tests: - Connection pool tests (initialization, distribution, recovery) - DOP tests (x-ms-dop-hint header parsing, parallelism calculation) - Bulk operation tests (CreateMultiple, UpdateMultiple, UpsertMultiple) - Throttle detection tests (tracker infrastructure, strategy selection) Phase 2 - Azure DevOps OIDC Authentication: - Add AzureDevOpsFederatedAuthenticationTests - Add SkipIfNoAzureDevOpsOidcAttribute - Update LiveTestConfiguration for Azure DevOps environment detection Phase 3 - Query and Action Tests: - QueryExecutionTests: paging, ordering, complex filters, FetchXML - AggregateQueryTests: COUNT, SUM, AVG, MIN, MAX, GROUP BY - CustomActionTests: CRUD requests, UpsertRequest, RetrieveMultipleRequest 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address bot review comments - unused variables, async dispose, comments - Rename Service_MultipleConcurrentCreates_AllSucceed to Service_MultipleSequentialCreates_AllSucceed (Gemini) - Use DisposeAsync instead of Dispose for pool in BulkOperationLiveTests cleanup (Gemini) - Fix DOP comments to say up to 52 instead of 50 for consistency with assertions (Copilot) - Fix folder reference in ThrottleDetectionLiveTests comment (Copilot) - Remove unused connectionId variable in ConnectionPoolLiveTests (Copilot/CodeQL) - Use discard pattern for unused pool variable in 5 test methods (Copilot/CodeQL) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: resolve InvalidateSeed disposing externally-owned clients + handle CLR crash in vuln check InvalidateSeed bug fix: - Pool was disposing seed clients that belong to IConnectionSource implementations - For ServiceClientSource, this disposed the externally-managed client while the source still held a reference to it, causing subsequent GetSeedClient() calls to return a disposed client - Fix: Let sources manage their own seed lifecycle via InvalidateSeed() - ConnectionStringSource: disposes and recreates on next GetSeedClient() - ServiceClientSource: no-op (caller owns the client lifecycle) Vulnerable packages check: - dotnet list package --vulnerable can crash with CLR error 0x80131506 on some SDK versions (JSON serializer bug in ReflectionEmitMemberAccessor) - Fix: Capture exit code, log warning on unexpected failures, always succeed - Actual vulnerability blocking is handled by dependency-review-action 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 5b5d5fc commit d82df51

File tree

14 files changed

+2457
-15
lines changed

14 files changed

+2457
-15
lines changed

.github/workflows/build.yml

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,30 @@ jobs:
4545
- name: Check for vulnerable packages
4646
shell: pwsh
4747
run: |
48+
# Run vulnerability check - capture output and exit code separately
49+
# Note: dotnet list package can crash with CLR errors on some SDK versions
4850
$output = dotnet list package --vulnerable --include-transitive 2>&1
51+
$dotnetExitCode = $LASTEXITCODE
4952
$output | Write-Host
5053
51-
if ($output -match "has the following vulnerable packages") {
54+
# Handle CLR crashes or other unexpected failures gracefully
55+
if ($dotnetExitCode -ne 0 -and $output -notmatch "has the following vulnerable packages") {
56+
Write-Host ""
57+
Write-Host "::warning::Vulnerable package check exited with code $dotnetExitCode (possible SDK bug, continuing)"
58+
}
59+
elseif ($output -match "has the following vulnerable packages") {
5260
Write-Host ""
5361
Write-Host "::warning::Vulnerable packages detected - review security advisories above"
5462
# Note: Not failing the build to avoid blocking on transitive dependencies
5563
# that require upstream fixes. Dependency-review-action will catch new vulns.
56-
} else {
64+
}
65+
else {
5766
Write-Host "No known vulnerabilities found in packages"
5867
}
5968
69+
# Always succeed - vulnerability blocking is handled by dependency-review-action
70+
exit 0
71+
6072
- name: Build
6173
run: dotnet build --configuration Release --no-restore
6274

src/PPDS.Dataverse/Pooling/DataverseConnectionPool.cs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,25 +1112,18 @@ public void InvalidateSeed(string connectionName)
11121112
}
11131113

11141114
// Remove from our seed cache
1115-
if (_seedClients.TryRemove(connectionName, out var oldSeed))
1115+
if (_seedClients.TryRemove(connectionName, out _))
11161116
{
11171117
_logger.LogWarning(
11181118
"Invalidating seed client for connection {ConnectionName} due to token failure. " +
11191119
"Next connection request will create fresh authentication.",
11201120
connectionName);
1121-
1122-
// Dispose the old seed
1123-
try
1124-
{
1125-
oldSeed.Dispose();
1126-
}
1127-
catch (Exception ex)
1128-
{
1129-
_logger.LogDebug(ex, "Error disposing old seed client for {ConnectionName}", connectionName);
1130-
}
11311121
}
11321122

1133-
// Invalidate the source's cached seed so GetSeedClient() creates a fresh one
1123+
// Invalidate the source's cached seed so GetSeedClient() creates a fresh one.
1124+
// The source owns the seed client and is responsible for disposal:
1125+
// - ConnectionStringSource: disposes and recreates on next GetSeedClient()
1126+
// - ServiceClientSource: no-op (externally-managed client, caller owns lifecycle)
11341127
var source = _sources.FirstOrDefault(s =>
11351128
string.Equals(s.Name, connectionName, StringComparison.OrdinalIgnoreCase));
11361129

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
using FluentAssertions;
2+
using Microsoft.Xrm.Sdk;
3+
using Microsoft.Xrm.Sdk.Messages;
4+
using Xunit;
5+
6+
namespace PPDS.Dataverse.IntegrationTests.Actions;
7+
8+
/// <summary>
9+
/// Tests for custom action and built-in message execution using FakeXrmEasy.
10+
/// </summary>
11+
/// <remarks>
12+
/// Note: FakeXrmEasy's open-source version (RPL-1.5) has limited message support.
13+
/// The following are NOT supported in the open-source version:
14+
/// - WhoAmIRequest (requires commercial license)
15+
/// - SetStateRequest (requires commercial license)
16+
/// - ExecuteMultipleRequest (requires commercial license)
17+
/// - Associate/Disassociate (requires metadata registration)
18+
///
19+
/// These tests cover what IS supported in the open-source version.
20+
/// For full coverage, live integration tests in PPDS.LiveTests cover the complete API.
21+
/// </remarks>
22+
public class CustomActionTests : FakeXrmEasyTestsBase
23+
{
24+
#region Request/Response Tests
25+
26+
[Fact]
27+
public void Execute_RetrieveRequest_ReturnsEntity()
28+
{
29+
// Arrange
30+
var entity = new Entity("account") { ["name"] = "Test Account" };
31+
var id = Service.Create(entity);
32+
33+
var request = new RetrieveRequest
34+
{
35+
Target = new EntityReference("account", id),
36+
ColumnSet = new Microsoft.Xrm.Sdk.Query.ColumnSet(true)
37+
};
38+
39+
// Act
40+
var response = (RetrieveResponse)Service.Execute(request);
41+
42+
// Assert
43+
response.Should().NotBeNull();
44+
response.Entity.Should().NotBeNull();
45+
response.Entity.Id.Should().Be(id);
46+
response.Entity.GetAttributeValue<string>("name").Should().Be("Test Account");
47+
}
48+
49+
[Fact]
50+
public void Execute_CreateRequest_ReturnsNewId()
51+
{
52+
// Arrange
53+
var request = new CreateRequest
54+
{
55+
Target = new Entity("account") { ["name"] = "New Account" }
56+
};
57+
58+
// Act
59+
var response = (CreateResponse)Service.Execute(request);
60+
61+
// Assert
62+
response.Should().NotBeNull();
63+
response.id.Should().NotBeEmpty();
64+
}
65+
66+
[Fact]
67+
public void Execute_UpdateRequest_ModifiesEntity()
68+
{
69+
// Arrange
70+
var entity = new Entity("account") { ["name"] = "Original" };
71+
var id = Service.Create(entity);
72+
73+
var request = new UpdateRequest
74+
{
75+
Target = new Entity("account", id) { ["name"] = "Updated" }
76+
};
77+
78+
// Act
79+
Service.Execute(request);
80+
81+
// Assert
82+
var retrieved = Service.Retrieve("account", id, new Microsoft.Xrm.Sdk.Query.ColumnSet(true));
83+
retrieved.GetAttributeValue<string>("name").Should().Be("Updated");
84+
}
85+
86+
[Fact]
87+
public void Execute_DeleteRequest_RemovesEntity()
88+
{
89+
// Arrange
90+
var entity = new Entity("account") { ["name"] = "To Delete" };
91+
var id = Service.Create(entity);
92+
93+
var request = new DeleteRequest
94+
{
95+
Target = new EntityReference("account", id)
96+
};
97+
98+
// Act
99+
Service.Execute(request);
100+
101+
// Assert
102+
var action = () => Service.Retrieve("account", id, new Microsoft.Xrm.Sdk.Query.ColumnSet(true));
103+
action.Should().Throw<Exception>();
104+
}
105+
106+
#endregion
107+
108+
#region Upsert Tests
109+
110+
// Note: FakeXrmEasy's UpsertRequest for new records (create path) has issues.
111+
// The update path works correctly. For full upsert coverage, see live tests.
112+
113+
[Fact]
114+
public void Execute_UpsertRequest_UpdatesExistingRecord()
115+
{
116+
// Arrange - Create existing record first
117+
var entity = new Entity("account") { ["name"] = "Original" };
118+
var id = Service.Create(entity);
119+
120+
var updateEntity = new Entity("account", id) { ["name"] = "Upserted Update" };
121+
var request = new UpsertRequest { Target = updateEntity };
122+
123+
// Act
124+
var response = (UpsertResponse)Service.Execute(request);
125+
126+
// Assert
127+
response.Should().NotBeNull();
128+
response.RecordCreated.Should().BeFalse("Existing record should be updated");
129+
130+
var retrieved = Service.Retrieve("account", id, new Microsoft.Xrm.Sdk.Query.ColumnSet(true));
131+
retrieved.GetAttributeValue<string>("name").Should().Be("Upserted Update");
132+
}
133+
134+
#endregion
135+
136+
#region RetrieveMultiple Request Tests
137+
138+
[Fact]
139+
public void Execute_RetrieveMultipleRequest_ReturnsResults()
140+
{
141+
// Arrange
142+
Service.Create(new Entity("account") { ["name"] = "Account 1" });
143+
Service.Create(new Entity("account") { ["name"] = "Account 2" });
144+
145+
var request = new RetrieveMultipleRequest
146+
{
147+
Query = new Microsoft.Xrm.Sdk.Query.QueryExpression("account")
148+
{
149+
ColumnSet = new Microsoft.Xrm.Sdk.Query.ColumnSet(true)
150+
}
151+
};
152+
153+
// Act
154+
var response = (RetrieveMultipleResponse)Service.Execute(request);
155+
156+
// Assert
157+
response.Should().NotBeNull();
158+
response.EntityCollection.Should().NotBeNull();
159+
response.EntityCollection.Entities.Should().HaveCount(2);
160+
}
161+
162+
[Fact]
163+
public void Execute_RetrieveMultipleRequest_WithFilter_ReturnsFilteredResults()
164+
{
165+
// Arrange
166+
Service.Create(new Entity("account") { ["name"] = "Alpha Corp" });
167+
Service.Create(new Entity("account") { ["name"] = "Beta Corp" });
168+
Service.Create(new Entity("account") { ["name"] = "Gamma Corp" });
169+
170+
var query = new Microsoft.Xrm.Sdk.Query.QueryExpression("account")
171+
{
172+
ColumnSet = new Microsoft.Xrm.Sdk.Query.ColumnSet("name"),
173+
Criteria = new Microsoft.Xrm.Sdk.Query.FilterExpression
174+
{
175+
Conditions =
176+
{
177+
new Microsoft.Xrm.Sdk.Query.ConditionExpression("name", Microsoft.Xrm.Sdk.Query.ConditionOperator.Equal, "Beta Corp")
178+
}
179+
}
180+
};
181+
182+
var request = new RetrieveMultipleRequest { Query = query };
183+
184+
// Act
185+
var response = (RetrieveMultipleResponse)Service.Execute(request);
186+
187+
// Assert
188+
response.EntityCollection.Entities.Should().HaveCount(1);
189+
response.EntityCollection.Entities[0].GetAttributeValue<string>("name").Should().Be("Beta Corp");
190+
}
191+
192+
#endregion
193+
194+
#region Batch Operations via Service Methods
195+
196+
[Fact]
197+
public void Service_CreateUpdateDelete_BatchWorkflow()
198+
{
199+
// This tests a typical batch workflow using individual operations
200+
201+
// Create
202+
var entity = new Entity("account") { ["name"] = "Batch Test" };
203+
var id = Service.Create(entity);
204+
id.Should().NotBeEmpty();
205+
206+
// Update
207+
var update = new Entity("account", id) { ["name"] = "Batch Updated" };
208+
Service.Update(update);
209+
210+
// Verify update
211+
var retrieved = Service.Retrieve("account", id, new Microsoft.Xrm.Sdk.Query.ColumnSet(true));
212+
retrieved.GetAttributeValue<string>("name").Should().Be("Batch Updated");
213+
214+
// Delete
215+
Service.Delete("account", id);
216+
217+
// Verify delete
218+
var action = () => Service.Retrieve("account", id, new Microsoft.Xrm.Sdk.Query.ColumnSet(true));
219+
action.Should().Throw<Exception>();
220+
}
221+
222+
[Fact]
223+
public void Service_MultipleSequentialCreates_AllSucceed()
224+
{
225+
// Arrange
226+
var entities = Enumerable.Range(1, 5).Select(i => new Entity("account")
227+
{
228+
["name"] = $"Concurrent Account {i}"
229+
}).ToList();
230+
231+
// Act
232+
var ids = entities.Select(e => Service.Create(e)).ToList();
233+
234+
// Assert
235+
ids.Should().HaveCount(5);
236+
ids.All(id => id != Guid.Empty).Should().BeTrue();
237+
238+
// Verify all records exist
239+
foreach (var id in ids)
240+
{
241+
var retrieved = Service.Retrieve("account", id, new Microsoft.Xrm.Sdk.Query.ColumnSet("name"));
242+
retrieved.Should().NotBeNull();
243+
}
244+
}
245+
246+
#endregion
247+
}

0 commit comments

Comments
 (0)