Skip to content

Handle duplicate setting, feature and permission records - #26209

Merged
EngincanV merged 3 commits into
devfrom
maliming/null-provider-key-duplicates
Oct 1, 2026
Merged

EngincanV merged 3 commits into
devfrom
maliming/null-provider-key-duplicates

Conversation

@maliming

@maliming maliming commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Resolve #26208

SettingManagementStore and FeatureManagementStore no longer throw on duplicate records: reads use the record FindAsync returns and saves delete the extra ones. Revoking a permission now deletes all matching grants.

Concurrent first saves can still insert duplicate rows. The unique index doesn't cover rows with a NULL ProviderKey, because EF Core adds an IS NOT NULL filter to it on SQL Server. On SQL Server, you can stop this at the database level:

  1. Remove the existing duplicates first, otherwise the migration will fail. This lists them with their values:
SELECT s.Id, s.Name, s.ProviderName, s.Value
FROM AbpSettings s
WHERE s.ProviderKey IS NULL
  AND EXISTS (
      SELECT 1 FROM AbpSettings d
      WHERE d.ProviderKey IS NULL
        AND d.Name = s.Name
        AND d.ProviderName = s.ProviderName
        AND d.Id <> s.Id)
ORDER BY s.Name, s.ProviderName;

Keep one row for each setting (the one with the value you want) and delete the others by Id.

  1. Add a filtered unique index in your own DbContext, after builder.ConfigureSettingManagement(), then add a new migration:
builder.Entity<Setting>(b =>
{
    b.HasIndex(x => new { x.Name, x.ProviderName })
        .IsUnique()
        .HasFilter("[ProviderKey] IS NULL");
});

@maliming
maliming requested a lite review from Copilot September 24, 2026 02:52
@maliming maliming added this to the 10.7-final milestone Sep 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@maliming
maliming marked this pull request as ready for review September 24, 2026 07:08
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.70954% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.95%. Comparing base (c80c6f8) to head (769ccc4).
⚠️ Report is 33 commits behind head on dev.

Files with missing lines Patch % Lines
...ment/ResourcePermissionManagementProvider_Tests.cs 0.00% 30 Missing ⚠️
...ssionManagement/ResourcePermissionManager_Tests.cs 0.00% 30 Missing ⚠️
...onManagement/PermissionManagementProvider_Tests.cs 0.00% 11 Missing ⚠️
...lo/Abp/FeatureManagement/FeatureManagementStore.cs 97.22% 0 Missing and 1 partial ⚠️
...lo/Abp/SettingManagement/SettingManagementStore.cs 97.95% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #26209      +/-   ##
==========================================
+ Coverage   49.90%   49.95%   +0.04%     
==========================================
  Files        3842     3843       +1     
  Lines      135458   135679     +221     
  Branches    10271    10282      +11     
==========================================
+ Hits        67597    67774     +177     
- Misses      65823    65867      +44     
  Partials     2038     2038              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

@maliming
maliming requested a review from EngincanV October 1, 2026 08:05
@EngincanV
EngincanV merged commit 0c0cce7 into dev Oct 1, 2026
6 checks passed
@EngincanV
EngincanV deleted the maliming/null-provider-key-duplicates branch October 1, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Global settings unique index does not prevent duplicate rows when ProviderKey is NULL (SQL Server), causing 500 on settings cache read

3 participants