DYN-9191: RecentFiles bug fix #16394
Conversation
Adds logic to refresh the recent file list when removed items are not contiguous, ensuring the UI stays in sync with the underlying data.
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9191
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the recent files cache management where removing the last item from the collection would throw an error when invalid paths exist and the maximum recent files limit has been reached.
- Replaces unsafe RemoveRange operation with bounds checking and fallback refresh logic
- Adds conditional logic to handle non-contiguous item removal scenarios
- Implements fallback to full list refresh when removed items don't align with collection boundaries
| var removedItemsCount = e.OldItems?.Count ?? 0; | ||
| if (recentFiles.Count > e.OldStartingIndex && recentFiles.Count >= removedItemsCount) | ||
| { | ||
| recentFiles.RemoveRange(e.OldStartingIndex, removedItemsCount); |
There was a problem hiding this comment.
What if you're starting to remove items from the starting index, but the (index from where you're starting to remove items) + removedItemsCount is greater than the total count of recentFiles, this could lead to an index out of range error, can't it?
There was a problem hiding this comment.
Or are you saying that this can never be the case?
There was a problem hiding this comment.
The remove range code is called when the RecentFiles collection is changed, specifically when the cache size goes beyond the allowed limit. This happen in 2 cases, user opens a 11th file when the limit is set to 10, in this case the new file is added to the list, size goes to 11, the last element at index 10 is removed, so the OldStartingIndex is 10, and removedItemsCount is 1.
The other case is when recent files limit is reduced from preferences, in this case lets say user updates the limit to 5, the OldStartingIndex is now 5, removedItemsCount is also 5.
Update method is here:
| } | ||
| else | ||
| { | ||
| RefreshRecentFileList(sender as IEnumerable<string>, true); |
There was a problem hiding this comment.
How and when will this case ever be true? Can the starting index ever be greater than or equal to the total size of the collection (recentFiles), or can the number of removed items be greater than the collection size?
There was a problem hiding this comment.
This can never be the case, unless there are corrupted paths in the recent files collection.
In which case the recentfiles collection items will be less that the index we will be removing.
But I am updating the code to not add non-existent files to recent files, which will make this moot.
(cherry picked from commit 2e34aca)
|
Successfully created backport PR for |
Purpose
When changing recent files cache, in case when max recent files has been reached, removing the last item from the collection would throw an error, if invalid paths exist.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Adds logic to refresh the recent file list when removed items are not contiguous, ensuring the UI stays in sync with the underlying data.
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of