diff --git a/eng/common/core-templates/job/helix-job-monitor.yml b/eng/common/core-templates/job/helix-job-monitor.yml index 53bbf74927e..7417afdf3ea 100644 --- a/eng/common/core-templates/job/helix-job-monitor.yml +++ b/eng/common/core-templates/job/helix-job-monitor.yml @@ -52,11 +52,23 @@ parameters: type: string default: https://helix.dot.net/ -# Helix API access token forwarded to the tool via the HELIX_ACCESSTOKEN environment variable. +# Helix API access token forwarded via HELIX_ACCESSTOKEN. Not forwarded when +# useEntraAuthentication is true. - name: helixAccessToken type: string default: '' +# Use a refreshable Entra credential instead of a PAT or anonymous access. +- name: useEntraAuthentication + type: boolean + default: false + +# Azure service connection ID authorized for Helix. Required when +# useEntraAuthentication is true. +- name: azureSubscription + type: string + default: '' + # Polling interval in seconds (--polling-interval-seconds). - name: pollingIntervalSeconds type: number @@ -141,6 +153,26 @@ jobs: - checkout: self fetchDepth: 1 + - ${{ if and(eq(parameters.useEntraAuthentication, true), eq(parameters.azureSubscription, '')) }}: + - pwsh: throw "azureSubscription must be set when useEntraAuthentication is true." + displayName: Validate Helix Entra authentication + + - ${{ if eq(parameters.useEntraAuthentication, true) }}: + - task: AzureCLI@2 + displayName: Initialize Helix Entra authentication + inputs: + azureSubscription: ${{ parameters.azureSubscription }} + addSpnToEnvironment: true + scriptType: pscore + scriptLocation: inlineScript + inlineScript: | + if ([string]::IsNullOrWhiteSpace($env:servicePrincipalId) -or [string]::IsNullOrWhiteSpace($env:tenantId)) { + throw "The Helix Azure service connection did not provide a service principal or tenant ID." + } + + Write-Host "##vso[task.setvariable variable=HelixEntraClientId]$env:servicePrincipalId" + Write-Host "##vso[task.setvariable variable=HelixEntraTenantId]$env:tenantId" + - ${{ if ne(parameters.toolNupkgArtifactName, '') }}: - task: DownloadPipelineArtifact@2 displayName: Download Helix Job Monitor artifact @@ -214,6 +246,7 @@ jobs: toolArgs=( --helix-base-uri '${{ parameters.helixBaseUri }}' + --use-entra-authentication '${{ parameters.useEntraAuthentication }}' --polling-interval-seconds '${{ parameters.pollingIntervalSeconds }}' --fail-on-failed-tests '${{ parameters.failWorkItemsWithFailedTests }}' --allow-no-helix-jobs '${{ parameters.allowNoHelixJobs }}' @@ -275,4 +308,9 @@ jobs: displayName: Monitor Helix Jobs env: SYSTEM_ACCESSTOKEN: $(System.AccessToken) - HELIX_ACCESSTOKEN: ${{ parameters.helixAccessToken }} + ${{ if eq(parameters.useEntraAuthentication, false) }}: + HELIX_ACCESSTOKEN: ${{ parameters.helixAccessToken }} + ${{ if eq(parameters.useEntraAuthentication, true) }}: + AZURESUBSCRIPTION_CLIENT_ID: $(HelixEntraClientId) + AZURESUBSCRIPTION_TENANT_ID: $(HelixEntraTenantId) + AZURESUBSCRIPTION_SERVICE_CONNECTION_ID: ${{ parameters.azureSubscription }} diff --git a/eng/common/core-templates/steps/send-to-helix.yml b/eng/common/core-templates/steps/send-to-helix.yml index ec7a2000399..62cce4d4ea2 100644 --- a/eng/common/core-templates/steps/send-to-helix.yml +++ b/eng/common/core-templates/steps/send-to-helix.yml @@ -4,7 +4,9 @@ parameters: HelixType: 'tests/default/' # required -- Helix telemetry which identifies what type of data this is; should include "test" for clarity and must end in '/' HelixBuild: $(Build.BuildNumber) # required -- the build number Helix will use to identify this -- automatically set to the AzDO build number HelixTargetQueues: '' # required -- semicolon-delimited list of Helix queues to test on; see https://helix.dot.net/ for a list of queues - HelixAccessToken: '' # required -- access token to make Helix API requests; should be provided by the appropriate variable group + HelixAccessToken: '' # optional -- legacy access token; not forwarded when HelixUseEntraAuthentication is true + HelixUseEntraAuthentication: false # optional -- use refreshable Entra authentication instead of a PAT or anonymous access + HelixAzureSubscription: '' # required when HelixUseEntraAuthentication is true -- Azure service connection ID authorized for Helix HelixProjectPath: 'eng/common/helixpublish.proj' # optional -- path to the project file to build relative to BUILD_SOURCESDIRECTORY HelixProjectArguments: '' # optional -- arguments passed to the build command HelixConfiguration: '' # optional -- additional property attached to a job @@ -32,12 +34,35 @@ parameters: continueOnError: false # optional -- determines whether to continue the build if the step errors; defaults to false steps: + - ${{ if and(eq(parameters.HelixUseEntraAuthentication, true), eq(parameters.HelixAzureSubscription, '')) }}: + - pwsh: throw "HelixAzureSubscription must be set when HelixUseEntraAuthentication is true." + displayName: Validate Helix Entra authentication + condition: ${{ parameters.condition }} + + - ${{ if eq(parameters.HelixUseEntraAuthentication, true) }}: + - task: AzureCLI@2 + displayName: Initialize Helix Entra authentication + inputs: + azureSubscription: ${{ parameters.HelixAzureSubscription }} + addSpnToEnvironment: true + scriptType: pscore + scriptLocation: inlineScript + inlineScript: | + if ([string]::IsNullOrWhiteSpace($env:servicePrincipalId) -or [string]::IsNullOrWhiteSpace($env:tenantId)) { + throw "The Helix Azure service connection did not provide a service principal or tenant ID." + } + + Write-Host "##vso[task.setvariable variable=HelixEntraClientId]$env:servicePrincipalId" + Write-Host "##vso[task.setvariable variable=HelixEntraTenantId]$env:tenantId" + condition: ${{ parameters.condition }} + - powershell: > $(Build.SourcesDirectory)\eng\common\msbuild.ps1 $(Build.SourcesDirectory)/${{ parameters.HelixProjectPath }} /restore /p:TreatWarningsAsErrors=false /p:EnableHelixJobMonitor=${{ parameters.UseHelixMonitor }} + /p:HelixUseEntraAuthentication=${{ parameters.HelixUseEntraAuthentication }} ${{ parameters.HelixProjectArguments }} /t:Test /bl:$(Build.SourcesDirectory)/artifacts/log/$(_BuildConfig)/SendToHelix.binlog @@ -49,7 +74,12 @@ steps: HelixBuild: ${{ parameters.HelixBuild }} HelixConfiguration: ${{ parameters.HelixConfiguration }} HelixTargetQueues: ${{ parameters.HelixTargetQueues }} - HelixAccessToken: ${{ parameters.HelixAccessToken }} + ${{ if eq(parameters.HelixUseEntraAuthentication, false) }}: + HelixAccessToken: ${{ parameters.HelixAccessToken }} + ${{ if eq(parameters.HelixUseEntraAuthentication, true) }}: + AZURESUBSCRIPTION_CLIENT_ID: $(HelixEntraClientId) + AZURESUBSCRIPTION_TENANT_ID: $(HelixEntraTenantId) + AZURESUBSCRIPTION_SERVICE_CONNECTION_ID: ${{ parameters.HelixAzureSubscription }} HelixPreCommands: ${{ parameters.HelixPreCommands }} HelixPostCommands: ${{ parameters.HelixPostCommands }} WorkItemDirectory: ${{ parameters.WorkItemDirectory }} @@ -76,6 +106,7 @@ steps: /restore /p:TreatWarningsAsErrors=false /p:EnableHelixJobMonitor=${{ parameters.UseHelixMonitor }} + /p:HelixUseEntraAuthentication=${{ parameters.HelixUseEntraAuthentication }} ${{ parameters.HelixProjectArguments }} /t:Test /bl:$(Build.SourcesDirectory)/artifacts/log/$(_BuildConfig)/SendToHelix.binlog @@ -87,7 +118,12 @@ steps: HelixBuild: ${{ parameters.HelixBuild }} HelixConfiguration: ${{ parameters.HelixConfiguration }} HelixTargetQueues: ${{ parameters.HelixTargetQueues }} - HelixAccessToken: ${{ parameters.HelixAccessToken }} + ${{ if eq(parameters.HelixUseEntraAuthentication, false) }}: + HelixAccessToken: ${{ parameters.HelixAccessToken }} + ${{ if eq(parameters.HelixUseEntraAuthentication, true) }}: + AZURESUBSCRIPTION_CLIENT_ID: $(HelixEntraClientId) + AZURESUBSCRIPTION_TENANT_ID: $(HelixEntraTenantId) + AZURESUBSCRIPTION_SERVICE_CONNECTION_ID: ${{ parameters.HelixAzureSubscription }} HelixPreCommands: ${{ parameters.HelixPreCommands }} HelixPostCommands: ${{ parameters.HelixPostCommands }} WorkItemDirectory: ${{ parameters.WorkItemDirectory }} @@ -108,4 +144,3 @@ steps: SYSTEM_ACCESSTOKEN: $(System.AccessToken) condition: and(${{ parameters.condition }}, ne(variables['Agent.Os'], 'Windows_NT')) continueOnError: ${{ parameters.continueOnError }} - diff --git a/global.json b/global.json index 8243d38c72e..990b8d1d95f 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "11.0.100-rc.1.26420.103", + "version": "11.0.100-rc.1.26425.128", "allowPrerelease": true, "rollForward": "latestMajor", "paths": [ @@ -13,7 +13,7 @@ "runner": "Microsoft.Testing.Platform" }, "tools": { - "dotnet": "11.0.100-rc.1.26420.103", + "dotnet": "11.0.100-rc.1.26425.128", "runtimes": { "dotnet": [ "$(MicrosoftNETCorePlatformsVersion)" diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 83b02b160f5..fa4eb4572b0 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -1277,15 +1277,12 @@ private void AddUniqueValueEdges() var rowIndexValueFactory = ((TableIndex)index).GetRowIndexValueFactory(); var (value, hasNullValue) = rowIndexValueFactory.CreateEquatableIndexValue(command); - if (command.EntityState == EntityState.Modified) - { - var (originalValue, _) = rowIndexValueFactory.CreateEquatableIndexValue(command, fromOriginalValues: true); - if (Equals(originalValue, value)) - { - continue; - } - } - + // Note: unlike the predecessor (first) pass above, we don't skip creating an edge here when the + // command's own index value appears unchanged. A predecessor is only ever registered in + // indexPredecessorsMap for a value it is actually releasing (see the check above), so if this + // command's value matches one, it genuinely needs to wait for that release, regardless of whether + // its own current/original value comparison (which can be unreliable, e.g. for attached entities + // whose original value was snapshotted after being modified) suggests no change occurred. if (value != null) { AddMatchingPredecessorEdge( diff --git a/test/Directory.Packages.props b/test/Directory.Packages.props index f9f89dc1b23..9c2b6fe7891 100644 --- a/test/Directory.Packages.props +++ b/test/Directory.Packages.props @@ -10,7 +10,7 @@ - + diff --git a/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs index 053f9e2c8c4..66cc126de82 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs @@ -119,6 +119,77 @@ public class Blog public string? Name { get; set; } } + [Theory, MemberData(nameof(IsAsyncData))] // Issue #38917 + public virtual async Task Reassigning_dependent_with_unique_FK_to_attached_entity_orders_updates_correctly(bool async) + { + var contextFactory = await InitializeNonSharedTest( + onModelCreating: mb => mb.Entity() + .HasOne(g => g.Color) + .WithOne(c => c.Group) + .HasForeignKey("GroupId"), + seed: async context => + { + var group0 = new WorkItemGroup38917 { Name = "G0" }; + var group1 = new WorkItemGroup38917 { Name = "G1" }; + var color0 = new RgbColor38917 { Id = "0x9a79e4", DisplayName = "red" }; + var color1 = new RgbColor38917 { Id = "0xb4e6b9", DisplayName = "turquoise" }; + + group0.Color = color0; + group1.Color = color1; + context.AddRange(group0, group1); + + await context.SaveChangesAsync(); + }); + + await ExecuteWithStrategyInTransactionAsync( + contextFactory, + async context => + { + var g1 = await context.Set() + .Include(g => g.Color) + .SingleAsync(g => g.Name == "G1"); + + var c0 = new RgbColor38917 { Id = "0x9a79e4" }; + context.Attach(c0); + g1.Color = c0; + + if (async) + { + await context.SaveChangesAsync(); + } + else + { + context.SaveChanges(); + } + }, + async context => + { + var g1 = await context.Set() + .Include(g => g.Color) + .SingleAsync(g => g.Name == "G1"); + Assert.Equal("0x9a79e4", g1.Color!.Id); + + var g0 = await context.Set() + .Include(g => g.Color) + .SingleAsync(g => g.Name == "G0"); + Assert.Null(g0.Color); + }); + } + + public class WorkItemGroup38917 + { + public Guid Id { get; set; } = Guid.NewGuid(); + public string Name { get; set; } = ""; + public RgbColor38917? Color { get; set; } + } + + public class RgbColor38917 + { + public string Id { get; set; } = ""; + public string? DisplayName { get; set; } + public WorkItemGroup38917? Group { get; set; } + } + [Theory, MemberData(nameof(IsAsyncData))] public virtual async Task Update_entity_with_not_loaded_property_excludes_column_from_SQL(bool async) { diff --git a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs index 401cbfd3746..d940261184c 100644 --- a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs +++ b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs @@ -599,6 +599,40 @@ public void BatchCommands_skips_unique_index_edges_for_unchanged_store_generated Assert.Equal(2, batch.ModificationCommands.Count); } + [Fact] // Issue #38917 + public void BatchCommands_orders_unique_index_release_before_reacquisition_with_unreliable_original_value() + { + var model = CreateModelWithUniqueIndexOnly(); + var configuration = CreateContextServices(model); + var stateManager = configuration.GetRequiredService(); + + // Releases "Test" by changing UniqueValue away from it. Uses a higher Id than the acquiring entry below so + // that the default (primary-key-based) command ordering alone would produce the wrong (unsafe) order, + // and only the dependency edge asserted by this test can produce the correct order. + var releasingEntry = stateManager.GetOrCreateEntry( + new FakeEntity { Id = 2, UniqueValue = "Other" }); + releasingEntry.SetEntityState(EntityState.Modified); + releasingEntry.SetOriginalValue(releasingEntry.EntityType.FindProperty(nameof(FakeEntity.UniqueValue))!, "Test"); + + // Simulates an attached entity whose original value snapshot ends up matching its (newly assigned) current + // value; it still needs to wait for another command to release "Test" before it can claim it. + var acquiringEntry = stateManager.GetOrCreateEntry( + new FakeEntity { Id = 1, UniqueValue = "Test" }); + acquiringEntry.SetEntityState(EntityState.Modified); + acquiringEntry.SetOriginalValue(acquiringEntry.EntityType.FindProperty(nameof(FakeEntity.UniqueValue))!, "Test"); + + var modelData = new UpdateAdapter(stateManager); + + var batches = CreateBatches([acquiringEntry, releasingEntry], modelData); + var batch = Assert.Single(batches); + + // The command releasing "Test" must be ordered before the command acquiring it, otherwise we'd get a + // unique constraint violation. + Assert.Equal( + [releasingEntry, acquiringEntry], + batch.ModificationCommands.Select(c => c.Entries.Single())); + } + [Fact] public void BatchCommands_creates_valid_batch_for_shared_table_added_entities() { @@ -1063,6 +1097,19 @@ private static IModel CreateSimpleFKModel() return modelBuilder.Model.FinalizeModel(); } + private static IModel CreateModelWithUniqueIndexOnly() + { + var modelBuilder = FakeRelationalTestHelpers.Instance.CreateConventionBuilder(); + + modelBuilder.Entity(b => + { + b.Ignore(c => c.RelatedId); + b.HasIndex(c => c.UniqueValue).IsUnique(); + }); + + return modelBuilder.Model.FinalizeModel(); + } + private static IModel CreateFKOneToManyModelWithGeneratedIds() { var modelBuilder = FakeRelationalTestHelpers.Instance.CreateConventionBuilder();