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

Treat symlinks to directories like directories #432

Merged
merged 1 commit into from Jul 16, 2021

Conversation

hupfdule
Copy link
Contributor

This allows symlinking to create additional subalbums.

Closes: #431

One drawback is that the files in the symlinked directories are scanned twice as they have different paths.
It would be nice if the applications identifies them as identical (e.g. based on an md5sum) and use the already scanned information for both subalbums (the real one and the symlink). But this is not part of this PR.

@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #432 (0129cb7) into master (2e4f83a) will decrease coverage by 0.89%.
The diff coverage is 58.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
- Coverage   42.37%   41.47%   -0.90%     
==========================================
  Files          86       90       +4     
  Lines        3212     3315     +103     
  Branches      348      348              
==========================================
+ Hits         1361     1375      +14     
- Misses       1734     1819      +85     
- Partials      117      121       +4     
Flag Coverage Δ
api 33.85% <58.33%> (-1.28%) ⬇️
ui 49.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/scanner/scanner_album.go 68.80% <40.00%> (-1.67%) ⬇️
api/scanner/scanner_user.go 55.63% <40.00%> (-0.89%) ⬇️
api/utils/utils.go 29.26% <71.42%> (ø)
api/utils/Throttle.go 0.00% <0.00%> (ø)
api/utils/environment_variables.go 0.00% <0.00%> (ø)
api/utils/Endpoints.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e4f83a...0129cb7. Read the comment docs.

@viktorstrate
Copy link
Member

Looks good, thank you. Would you be up for writing some tests for the IsDirSymlink function?
If you don't want to, I'll still gladly merge and add it myself later.

Test setup:

  • Make a api/utils/utils_test.go and add the following
func TestMain(m *testing.M) {
	os.Exit(test_utils.IntegrationTestRun(m))
}
  • Then for each test within the api/ui directory that uses the file system add the line test_utils.FilesystemTest(t). This marks it as an integration test.

Take a look at scanner_test.go for inspiration.

@hupfdule hupfdule force-pushed the dir-symlinks branch 2 times, most recently from 0d6a881 to 48ff06d Compare July 15, 2021 20:56
@hupfdule
Copy link
Contributor Author

OK, I tried writing a test case for that method.

I am not very used to Go, therefore the test may not be optimal. Feel free to adapt it if necessary.

@viktorstrate
Copy link
Member

The tests look great, exactly what I imagined!
Although I think you forgot to clean up the TODO on line 20?

This allows symlinking to create additional subalbums.

Closes: photoview#431
@hupfdule
Copy link
Contributor Author

Although I think you forgot to clean up the TODO on line 20?

You're absolutely right. Fixed it now.

@viktorstrate
Copy link
Member

Looks good, thank you!

@viktorstrate viktorstrate merged commit 0fb434e into photoview:master Jul 16, 2021
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.

Symlinks to directories should be handled like directories, not files
2 participants