Fix #803 (again) ensure the self referencing nodes are removed #809
Merged
Conversation
…e last time!).
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issue #803 by properly removing self-referencing nodes from the graph structure during topological sorting. The fix prevents nodes from referencing themselves, which was causing issues in the previous implementation.
- Filters out self-references when building the graph edges to prevent circular dependencies
- Removes unnecessary graph cleaning step that was incorrectly filtering nodes
- Pre-allocates result list capacity for better performance
Comments suppressed due to low confidence (1)
uSync.BackOffice/SyncHandlers/SyncHandlerContainerBase.cs:1
- The removed code was filtering to keep only self-referencing edges (
x.Node == x.Edge) instead of removing them. This would have passed only circular references to TopologicalSort, which is incorrect behavior that the current fix addresses properly.
using Microsoft.Extensions.Logging;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This was referenced Oct 20, 2025
This was referenced Oct 27, 2025
alexsee
pushed a commit
to alexsee/umbraco-container
that referenced
this pull request
Dec 6, 2025
[//]: # (dependabot-start)⚠️ **Dependabot is rebasing this PR**⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Updated [uSync](https://github.com/KevinJump/uSync) from 16.0.5 to 16.1.0. <details> <summary>Release notes</summary> _Sourced from [uSync's releases](https://github.com/KevinJump/uSync/releases)._ ## 16.1.0 This is a minor update to uSync for Umbraco v16, it contains feature updates to how BlockList/Grid merges are reported and the ability to merge uSync files for multiple items into single files so they can be read in one block from the server (no UI for this, feature will likely be enabled via the uSync.Cli). ## What's Changed * V16/block converters two by @KevinJump in KevinJump/uSync#822 * pass latest content version between passes. by @KevinJump in KevinJump/uSync#823 * V16/firstboot on startup conflict fix by @KevinJump in KevinJump/uSync#824 * Add operation security filter so swagger auth is allowed. by @KevinJump in KevinJump/uSync#825 * Share second pass actions across commonn IContentBase objects by @KevinJump in KevinJump/uSync#826 * Rix #827 - Removal's code was trying to modify array inside loop of array by @KevinJump in KevinJump/uSync#828 * V16/multiple nodes per file by @KevinJump in KevinJump/uSync#829 **Full Changelog**: KevinJump/uSync@v16.0.8...v16.1.0 This will be the last 'feature' release of uSync for Umbraco v16 as we move on to supporting and targeting Umbraco v17 (see https://docs.jumoo.co.uk/SupportLifecycles#Short-Term-Support-STS-Releases) ## 16.0.8 This is a patch release of uSync for Umbraco, it contains some fixes issues around imports on startup and UI messages. ## What's Changed * update package.json dependencies. by @KevinJump in KevinJump/uSync#819 * Offer more granular updates to the ordering process by @KevinJump in KevinJump/uSync#820 * Get Correct Content version on second pass events. by @KevinJump in KevinJump/uSync#821 **Full Changelog**: KevinJump/uSync@v16.0.7...v16.0.8 ## 16.0.7 This is a patch release for uSync for Umbraco v16 - it contains some fixes and updates based on reported issues. ## What's Changed * Some string changes to reduce some allocations here and there. by @KevinJump in KevinJump/uSync#808 * Fix #803 (again) ensure the self referencing nodes are removed by @KevinJump in KevinJump/uSync#809 * only check for legacy when the default folder hasn't been changed. by @KevinJump in KevinJump/uSync#810 * V16/dashboard tweaks by @KevinJump in KevinJump/uSync#811 * normalize string media paths into JSON croppers for consistancy. by @KevinJump in KevinJump/uSync#812 * Fix: Compiled Views Content by @KevinJump in KevinJump/uSync#813 * move away from uui-table-row by @KevinJump in KevinJump/uSync#814 * v16 fix of #815 by @KevinJump in KevinJump/uSync#818 **Full Changelog**: KevinJump/uSync@v16.0.5...v16.0.7 Commits viewable in [compare view](KevinJump/uSync@v16.0.5...v16.1.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) You can trigger a rebase of this PR by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> > **Note** > Automatic rebases have been disabled on this pull request as it has been open for over 30 days. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Last time we where keeping the nodes that matched (as opposed to removing them).
this fix does that, and does it in the loop so we don't have to create a new version of the list once we've done it.