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

add duplicate files validation when pick image or files or directory #904

Conversation

programmermager
Copy link
Contributor

@programmermager programmermager commented Nov 16, 2023

linked to #903

I try to add validation for already selected files from menu options file, media or folder.

Before

MacOs

before_mac.mp4

Android

before_android.mp4

After

MacOs

after_mac.mp4

Android

after_android.mp4

@Tienisto
Copy link
Member

Tienisto commented Nov 16, 2023

Thank you. I think we need to check the absolute path and not the base path since users can add a directory (with its sub directories). There might be the case where you have 2 files with the same name but in different directories

@programmermager
Copy link
Contributor Author

programmermager commented Nov 16, 2023

Thank you. I think we need to check the absolute path and not the base path since users can add a directory (with its sub directories). There might be the case where you have 2 files with the same name but in different directories

ah i see.

The reason why i use the basename because the path from gallery and file is different, although the file is same but it returns different path.

let me think for better way

@programmermager
Copy link
Contributor Author

programmermager commented Nov 16, 2023

@Tienisto I believe it's not possible to compare using an absolute path when the files come from the file picker and media picker. They are entirely different even if the files are the same. Below, I have attached the paths provided by the file picker and media picker.

pathimage

The green line represents the absolute path from the file picker, while the yellow one is from the media/gallery. It's the same file; the only thing that hasn't changed is the file name.

I would like to suggest continuing to use the basename for validation, but with additional comparison based on file size. Perhaps comparing only the file names could lead to errors, but if we compare based on both the file name and size parameters, I believe it would have high accuracy, unless there is one duplicated file.

WDYT?

I tried the above way and this is the result :

The images of the driver and the chair share the same name, but they are distinct files, as indicated by their different file sizes.

compare_title_and_size.mp4

@WubTheGame
Copy link

WubTheGame commented Nov 16, 2023

@Tienisto I believe it's not possible to compare using an absolute path when the files come from the file picker and media picker. They are entirely different even if the files are the same. Below, I have attached the paths provided by the file picker and media picker.

pathimage The green line represents the absolute path from the file picker, while the yellow one is from the media/gallery. It's the same file; the only thing that hasn't changed is the file name.

I would like to suggest continuing to use the basename for validation, but with additional comparison based on file size. Perhaps comparing only the file names could lead to errors, but if we compare based on both the file name and size parameters, I believe it would have high accuracy, unless there is one duplicated file.

WDYT?

Wouldn't comparing based on file hash be better? It'd would cover everything else about the file too. If checking for an absolute duplicate, I think that'd be the way to go.

Also wondering, why does Localsend copy files into a temporary directory of its own before sending them? What's the point of that? Why not just collect paths and grab from them at the moment of transfer? I can see that being a problem if you're low on storage.

Not a developer, so please pardon any misunderstandings.

@programmermager
Copy link
Contributor Author

programmermager commented Nov 16, 2023

@Tienisto I believe it's not possible to compare using an absolute path when the files come from the file picker and media picker. They are entirely different even if the files are the same. Below, I have attached the paths provided by the file picker and media picker.
pathimage
The green line represents the absolute path from the file picker, while the yellow one is from the media/gallery. It's the same file; the only thing that hasn't changed is the file name.
I would like to suggest continuing to use the basename for validation, but with additional comparison based on file size. Perhaps comparing only the file names could lead to errors, but if we compare based on both the file name and size parameters, I believe it would have high accuracy, unless there is one duplicated file.
WDYT?

Wouldn't comparing based on file hash be better? It'd would cover everything else about the file too. If checking for an absolute duplicate, I think that'd be the way to go.

Also wondering, why does Localsend copy files into a temporary directory of its own before sending them? What's the point of that? Why not just collect paths and grab from them at the moment of transfer? I can see that being a problem if you're low on storage.

Not a developer, so please pardon any misunderstandings.

I think it's come from the file picker library used

@Tienisto
Copy link
Member

I think we should only exclude the same absolute file (when selected from the file picker) for now - which is still an improvement over the current version.

Only comparing the file size would cause false positives in rare cases which essentially a bug.

Calculating the hash would significantly increase the time during file selection (which is already quite slow sometimes).

@programmermager
Copy link
Contributor Author

btw the latest i tried to comparing the basename and file size, not only the size.

Are you saying it's acceptable for the same file to be included in the list of files to be sent when selected from the files and media menu?

@Tienisto
Copy link
Member

Yeah, comparing file name and file size just feels not right.

Ideally, file name and hash would be better but it's too slow.

To me, only applying the validation to "File" is enough if we use absolute paths.

That's just my opinion. I am open for other opinions.

@programmermager
Copy link
Contributor Author

I tried to perform a comparison using bytes. The problem wasn't particularly noticeable for image files. However, when selecting large files, such as videos and in large quantities, it takes too much time. So that options i believe we both doesnt agree to use that.

So, if you believe that a comparison based on name and size is not effective, it seems like the best option at the moment is an absolute path comparison. The drawback is that if the same file is retrieved using different methods, for example, via files and media, it will still be accepted and added to the list of files sent.

i will push new commit for comparation using absolute path

@Tienisto
Copy link
Member

Thank you. This should be fine for now.

@Tienisto Tienisto merged commit e59a060 into localsend:main Nov 17, 2023
@programmermager
Copy link
Contributor Author

My Pleasure, Hopefully I can contribute more here.

I have some ideas that might be implemented in LocalSend

@programmermager programmermager deleted the feat/validation-duplicate-files-when-send branch November 20, 2023 08:56
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.

None yet

3 participants