Skip to content
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 Readdir() and Name() #59

Closed
wants to merge 2 commits into from
Closed

Fix Readdir() and Name() #59

wants to merge 2 commits into from

Conversation

dkumor
Copy link

@dkumor dkumor commented Feb 16, 2019

Statik's Readdir did not behave in the same way as a http.FileSystem's Readdir. Similarly, the Name function in http.FileSystem returned the base name, and not a full file path.

This commit fixes both issues, making both Name and Readdir behave in the same way as when running the raw filesystem directly using http.Dir()

The commit is an extension of #55, which was a first step towards replicating the correct behaviour, but failed to account for the Name() issue, and did not include the necessary extra Trim to get rid of leading slash.

The corresponding tests ensure that the results are identical for http.Dir() and statik.

Closes #58

Statik's Readdir did not behave in the same way as a http.FileSystem's Readdir. Similarly, the Name function in http.FileSystem returned the base name, and not a full file path.

This commit fixes both issues, making both Name and Readdir behave in the same way as in http.Dir()
@dkumor
Copy link
Author

dkumor commented Feb 16, 2019

Not sure why travis succeeded on one test, yet failed on another. The failure happens in Open, which works identically in this version (the file map keys were not changed).

This is some form of timing issue, which I am able to intermittently repeat by running the tests many times. I will try to see why it is happening later today.

Folders were created in a for loop iterating over map keys. This led to an instability in newer golang versions, since adding to a map while looping over it can either produce the key again or not produce it.

In order to successfully generate a folder structure, it was implicitly assumed that each folder has a file, which would not require recursing from a file down to its ancestor folders.

When running without such a requirement, the elements added in the loop sometimes recursed, if their key was produced again in the loop, and sometimes did not. This caused an intermittent issue with tests sometimes failing to find a folder.
@dkumor
Copy link
Author

dkumor commented Feb 16, 2019

It turned out the issue was that folders were added in a for loop over map keys, and sometimes recursed into their ancestors (if the added key was produced again in the loop) and sometimes did not (if it wasn't).

This issue was initially hidden from tests because each folder had at least one file in it, meaning that recursive creation of parents was not necessary. The tests I added for Readdir had this structure, which exposed this secondary bug.

The above commit fixes it, meaning that this PR fixes 3 issues:

  1. Name() matches http.FileSystem
  2. Readdir() matches http.FileSystem
  3. Fixed instability when file system has multiple embedded directories with no files.

@dkumor
Copy link
Author

dkumor commented Mar 20, 2019

Fixed in #61

@dkumor dkumor closed this Mar 20, 2019
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.

Readdir returns sub-subfolders
1 participant