-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix getNodeForPath caching #1545
fix getNodeForPath caching #1545
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1545 +/- ##
============================================
+ Coverage 97.22% 97.24% +0.01%
- Complexity 2834 2835 +1
============================================
Files 175 175
Lines 9018 8849 -169
============================================
- Hits 8768 8605 -163
+ Misses 250 244 -6 ☔ View full report in Codecov by Sentry. |
c8ee60b
to
0bf4dc6
Compare
@icewind1991 - you rock! See nextcloud/server#42143 |
For those that don't have root access to their NC installs, which version can we expect the fix to arrive in? Thanks! |
$node = $this->rootNode; | ||
|
||
// look for any cached parent and collect the parts below the parent | ||
$parts = []; | ||
$remainingPath = $path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: we know that $path
is not the empty string, because of the check a few lines above.
So the call to Uri\split()
below will always pass a non-empty string.
So we don't have to worry about the fact that Uri\split
returns [null, null]
when passed and empty string.
Just noting this, because I saw that Uri\split
can return [null, null]
and I was concerned about that - it would cause it to be called again with null
and explode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@staabm @DeepDiver1975 any comment? or merge and release? |
* fix getNodeForPath caching * add test that cached parent nodes are used in Tree::getNodeForPath
fix: getNodeForPath caching (#1545) [4.6]
The change #1060 from the recursive to iterative
getNodeForPath
means that any cached parent node isn't used anymore.This adds some logic to find the nearest parent that is cached and start the iterative logic from there.