Skip to content

Protect user updates and clean up orphaned private views - #2631

Open
niemyjski wants to merge 15 commits into
issue/external-account-statefrom
issue/user-record-concurrency
Open

niemyjski wants to merge 15 commits into
issue/external-account-statefrom
issue/user-record-concurrency

Conversation

@niemyjski

@niemyjski niemyjski commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

User saves use native optimistic concurrency, membership changes honor patch outcomes, and failed account transfers preserve revocation without speculative ownership restoration. Membership grants and private-view cleanup share a Foundatio lock; the existing cleanup job skips busy users and checks access while holding the lock. Predefined-view initialization updates only its metadata, preserving concurrent invitations. Fixes #2672.

Validation: build passed with zero warnings/errors and all 105 focused isolated tests passed. The real invitation/cleanup regression failed before the coordination fix; final CI is pending on 98032430f. The PR is rebased onto #2630 head a7b95bf33.

Stack dependency: merge #2630 first; #2629 is already merged. An ambiguous transfer failure can leave an account unlinked; retry coverage verifies recovery. Organization creation rolls back definite repository rejections and preserves state after ambiguous failures.

Verification and implementation details
  • Removed custom user-save and legacy-cache read overrides, the temporary cache-version test, and the user-specific API serializer modifier. Elasticsearch supplies concurrency metadata for existing records; no migration or resave is required. Old cache entries expire normally.
  • User versioning follows the documented Foundatio pattern: a normal IVersioned implementation with public string Version { get; set; } = String.Empty;. Elasticsearch supplies persisted versions; new users serialize an empty version. There is no explicit interface wrapper or API serializer special case.
  • Predefined-view initialization records its existing definitions hash using native PartialPatch, preserving concurrent invitations and unrelated organization data. The real regression commits an invitation and another data entry while predefined views are being created, then verifies both survive and invite-token lookup succeeds. It failed before the fix with an empty invite collection and passed on the final revision. This follows Foundatio's partial-update guidance without adding organization versioning or another model field.
  • Verified that the repository's single and bulk delete requests do not include document-version preconditions. Implementing IVersioned does not introduce the stale-delete conflict claimed by the final review comment; no speculative deletion patch was added.
  • Membership handlers use named patch results and normal cached follow-up reads. Retries recheck current cached membership and administrator access before revocation and cleanup; nonmember global administrators retain their legitimate views, including when canceling an invite. Real HTTP tests cover retries, invite cancellation, and membership restored during cleanup. Real repository tests warm ID and email caches, patch membership, and verify current data. Removed custom repository classes and lock mocks from integration tests; failure injection is confined to handler unit tests.
  • The existing cleanup job scans private views in 500-record pages. It removes views belonging to deleted users, missing organizations, and former members. Shared views, administrators, disabled members with membership, and views in another accessible organization remain intact.
  • The cleanup regression crosses the page boundary with 503 orphaned views and five retained views. Additional real-repository tests change visibility after the scan and restore membership during both the initial and final organization lookups. Cleanup reads the organization before the final owner-access check so grants committed during that lookup are observed. Deletion filters on the scanned view IDs, owner, organization, and private visibility.
  • The final organization-lookup case failed before its read-order fix, while the earlier lookup case passed. Both pass on the final revision. The subsequent invitation-acceptance regression pauses a real HTTP grant during persistence, runs the real cleanup job, finishes the grant, and verifies membership and the old private view both survive. It failed on the missing-view assertion before coordination and passed after the fix. Existing-user adds, invite acceptance, organization creation, and global-admin grants now share a per-user Foundatio lock with background cleanup and foreground revocation cleanup. The job skips busy users for its next run; its final access read and deletion hold the same lock. This follows Foundatio lock guidance, with no persisted coordination field or custom repository.
  • Organization deletion removes members while the requester is available, then persists the existing soft-delete marker before cleaning up resources. A member-write conflict returns 409 before marking the organization deleted; the real HTTP regression retries and verifies the requester survives final cleanup. The job uses its existing cleanup behavior without persisting requester state or adding a model field. Unit tests cover marker-write and token-cleanup failures. No migration is required.
  • Forgot-password handles native version conflicts internally, returns the same success response as an unknown email, and sends no uncommitted reset token. The regression performs a real concurrent profile write during persistence and checks the HTTP response, preserved data, and mail queue.
  • Organization creation/deletion, membership add/remove, invite-bearing login/signup, and affected user mutations declare their 409 responses. Forgot-password no longer advertises 409. OpenAPI contract assertions and the generated snapshot cover these responses.
  • Integrated the latest parent branch, preserving its external-account validation, locks, uncached admission lookup, and session-generation checks. Its stale-email-cache regression uses the real cache and native repositories without a custom subclass. Concurrent-login and canceled-lookup integration tests use production repositories and locks; unavailable-lock behavior is covered in a unit test.
  • Integration tests use the standard Aspire fixture and localhost Elasticsearch. Local API execution was blocked by an unhealthy Docker daemon; the reported real integration results come from hosted CI. CI shard discovery, assertions, retry settings, and flaky-test gates remain unchanged. Late cache publication is separately tracked in Foundatio.Repositories #323.

@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 5bc84d1 to 179c281 Compare October 2, 2026 04:10
@niemyjski
niemyjski added this pull request to stack #2642 October 5, 2026 02:11
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 179c281 to 032664f Compare October 13, 2026 02:39
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 032664f to 57f409f Compare October 13, 2026 02:41
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 57f409f to 975b25e Compare October 13, 2026 02:58
@niemyjski
niemyjski removed this pull request from stack #2642 October 13, 2026 03:05
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 975b25e to 793b7c9 Compare October 13, 2026 03:05
@niemyjski
niemyjski added this pull request to stack #2659 October 13, 2026 03:05
@niemyjski
niemyjski removed this pull request from stack #2659 October 13, 2026 12:26
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 793b7c9 to 2c36ec1 Compare October 13, 2026 12:26
@niemyjski
niemyjski added this pull request to stack #2660 October 13, 2026 12:27
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 2c36ec1 to a0d6b0e Compare October 13, 2026 12:40
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from a0d6b0e to aa27175 Compare October 13, 2026 13:33
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from aa27175 to c2ffd53 Compare October 13, 2026 13:55
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from c2ffd53 to d307b6a Compare October 14, 2026 03:03
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from d307b6a to 8e94661 Compare October 14, 2026 03:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f60c014ff0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Exceptionless.Core/Services/OrganizationService.cs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 376ed2bea4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a66489346a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Exceptionless.Core/Services/OrganizationService.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af59bd303b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Exceptionless.Core/Models/User.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ccc8e1786

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Exceptionless.Core/Models/User.cs
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 1a51885 to 730c034 Compare October 20, 2026 00:10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5995389b47

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Exceptionless.Core/Jobs/CleanupDataJob.cs
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch 2 times, most recently from 4ccf69e to 054b8f6 Compare October 20, 2026 01:35
@niemyjski
niemyjski force-pushed the issue/user-record-concurrency branch from 054b8f6 to 9803243 Compare October 20, 2026 01:39
@github-actions

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
Exceptionless.AppHost 23% 23% 128 ❌
Exceptionless.Core 78% 70% 10987 ✔
Exceptionless.Insulation 51% 43% 370 ➖
Exceptionless.Web 86% 71% 9334 ✔
Summary 80% (28606 / 35560) 69% (14179 / 20404) 20819 ✔

This branch has not been deployed

No deployments
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.

Correct user membership patch outcomes and external-account transfer recovery

1 participant