You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@commons.apache.org by "ilang (via GitHub)" <gi...@apache.org> on 2026/04/08 19:36:16 UTC

[PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

ilang opened a new pull request, #761:
URL: https://github.com/apache/commons-vfs/pull/761

   ## Summary
   
   - Internal VFS navigation (`getParent()`, `getRoot()`, `getChildren()` child resolution, symlink resolution) triggers `CacheStrategy.ON_RESOLVE` refresh, causing redundant FTP/SFTP operations
   - This was always wasteful (2 LIST commands instead of 1 for sibling file lookups), but became O(N) after PR #758 added unconditional `childMap = null` to `FtpFileObject.refresh()`
   - A directory with 50 files produces ~50 LIST commands instead of 1
   
   ## Fix
   
   1. Add `resolveFileInternal()` that skips the ON_RESOLVE refresh for internal navigation
   2. After a fresh directory listing, FTP and SFTP providers propagate metadata to cached children in-place (preserving object identity)
   
   ## Test plan
   
   - [x] `FtpGetChildrenListCommandTest`: verifies `findFiles()` issues exactly 1 LIST with ON_RESOLVE (50 files)
   - [x] `SftpGetChildrenListCommandTest`: verifies refresh + `findFiles()` returns fresh children
   - [x] All existing tests pass (3208 tests, 0 failures)
   - [x] Checkstyle clean
   
   JIRA: https://issues.apache.org/jira/browse/VFS-862


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "ilang (via GitHub)" <gi...@apache.org>.

ilang commented on PR #761:
URL: https://github.com/apache/commons-vfs/pull/761#issuecomment-4216526015

   Thanks @garydgregory , see my comment above, it is a bug in test (or actually the old jackrabbit that the test uses) not in the code, I create a fix to bypass it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.

garydgregory merged PR #761:
URL: https://github.com/apache/commons-vfs/pull/761


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.

garydgregory commented on PR #761:
URL: https://github.com/apache/commons-vfs/pull/761#issuecomment-4209622115

   Hello @ilang 
   
   Please see the test failures.
   
   TY!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.

garydgregory commented on PR #761:
URL: https://github.com/apache/commons-vfs/pull/761#issuecomment-4225066561

   Hello @ilang 
   
   Thank you for the update. I don't see the need for busy work and creating another PR for what's already here.
   
   I'll review again tomorrow. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "ilang (via GitHub)" <gi...@apache.org>.

ilang commented on PR #761:
URL: https://github.com/apache/commons-vfs/pull/761#issuecomment-4216495303

This PR now includes a fix for the Jackrabbit 1.x WebDAV test failure (`testFileCreate: expected 0 but was 112`) that was caused by the `getParent()` change.

**What happened:** The embedded Jackrabbit 1.x test server bundles Jetty 6.x, which does not drain unconsumed request bodies on 404 responses. When VFS sends a PROPFIND (112-byte XML body) to check if a file exists and gets a 404, then reuses the same persistent connection for a PUT to create the file, Jetty reads the stale PROPFIND bytes as the PUT body — storing the XML as file content.

Before this PR, `getParent()` used `resolveFile()` which triggered an extra PROPFIND to the parent directory. That extra request happened to flush the stale bytes from the connection, masking the server bug. With `resolveFileInternal`, the PUT follows the 404 immediately, exposing it.

This is a known class of Jetty issue — [jetty#651](https://github.com/jetty/jetty.project/issues/651), [jetty#4117](https://github.com/jetty/jetty.project/issues/4117), [jetty#6168](https://github.com/jetty/jetty.project/issues/6168). Fixed in Jetty 9.4+.

**Fix:** Add `Connection: close` to the Jackrabbit 1.x test HttpClient configuration (second commit). No production code changes. Jackrabbit 2 tests already pass without any workaround.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "ilang (via GitHub)" <gi...@apache.org>.

ilang commented on PR #761:
URL: https://github.com/apache/commons-vfs/pull/761#issuecomment-4209107878

   Hi @garydgregory 
   I tried to fix PR #758 with some easy fix, but I think the root cause is the refresh behavior, which I try to fix in this PR.
   So this is my stab at fixing it.
   I am aware that it is a bit of a larger fix, and I am new here, but this is the simplest and most idiomatic root I found.
   I summarized in the Jira that it might have affect on existing behavior, but this is if someone relates on existing bugs.
   
   If you don't accept this fix, you should probably revert PR #758 or fix it in a better way.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [VFS-862] Fix ON_RESOLVE triggering refresh on internal navigation [commons-vfs]

Posted by "ilang (via GitHub)" <gi...@apache.org>.

ilang commented on PR #761:
URL: https://github.com/apache/commons-vfs/pull/761#issuecomment-4224557943

   You're right about the SFTP test — I've removed it.
   
   The SFTP code change (`resolveFileInternal` in `doListChildrenResolved()` + `injectType(null)` after `setStat()`) applies the same `resolveFileInternal` principle consistently, but has no server-side observable difference. SFTP's `doListChildrenResolved()` pushes metadata to each child via `setStat()` immediately after resolution, so the ON_RESOLVE refresh is redundant client-side work — clearing `attrs` that is immediately repopulated. Unlike FTP, where the cascade causes real network LIST commands, the SFTP optimization is purely client-side and cannot be distinguished by a server-side test.
   
   If you prefer, I can drop the SFTP changes from this PR and submit them separately, keeping this PR focused on the FTP-observable fix.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org