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

Verify new Location is not parent or subdir of existing path #254

Merged
merged 3 commits into from
Nov 6, 2021

Conversation

robbevp
Copy link
Member

@robbevp robbevp commented Sep 19, 2021

Verifies whether a new location is a parent or subdirectory of an existing location.

I think it makes sense to add this validation, since adding two paths that contain the same files will cause tracks to show up twice.

The validation is added in the simplest way possible, by checking whether the DB already has path that either starts with the new path or where new path starts the same as an existing string. This obviously has two drawbacks:

  • If a relative path is added, this won't work (though I'm not sure what if relative paths work for the RescanRunner)
  • If the server admin uses symlinks, we might miss some things.
  • I've added tests relevant to my changes.

@robbevp robbevp added the enhancement New feature or request label Sep 19, 2021
@robbevp robbevp self-assigned this Sep 19, 2021
@robbevp robbevp force-pushed the enhc/verify-paths-parent-subdir branch from 9a93fc4 to 3f2e433 Compare September 19, 2021 11:56
Copy link
Member

@chvp chvp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the casing thing makes sense. Paths on sane OS's/filesystems are case-sensitive, so /var and /Var are different things.

@robbevp robbevp requested a review from chvp October 2, 2021 12:02
@chvp chvp enabled auto-merge (squash) November 6, 2021 08:57
@chvp chvp merged commit 31285ca into main Nov 6, 2021
@chvp chvp deleted the enhc/verify-paths-parent-subdir branch November 6, 2021 08:59
@chvp
Copy link
Member

chvp commented Nov 6, 2021

I was a bit too eager with merging this. The following test does not succeed:

  test 'should be able to add similar siblings' do
    sibling = build(:location, path: '/var/parent2')
    assert sibling.valid?
  end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants