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

File ids must stay stable #13310

Closed
3 tasks done
PVince81 opened this issue Jan 13, 2015 · 19 comments
Closed
3 tasks done

File ids must stay stable #13310

PVince81 opened this issue Jan 13, 2015 · 19 comments

Comments

@PVince81
Copy link
Contributor

Here are a few features that rely on file ids:

  • favorites feature
  • shares
  • sync client to detect file move operations. Failing to do so will cause redownloads. (CC @dragotin)

Currently there are several known situations in which the file id changes:

We need to make the necessary changes in the code to strive and keep the file ids stable.

@DeepDiver1975 @icewind1991 @schiesbn

@DeepDiver1975
Copy link
Member

Thanks for the summary - let's attack this in OC8.1

@DeepDiver1975 DeepDiver1975 added this to the 8.1-next milestone Jan 13, 2015
@DeepDiver1975
Copy link
Member

OC8.1

which is about to explode already ......

@PVince81
Copy link
Contributor Author

I wrote some unit tests (in the spirit of "test first") here: #13312

@dragotin
Copy link
Contributor

Especially case 2) File is moved between storages (from/to external storage) is bad as the client and also the user does not care about the storage type or knows about it. That can be considered a plain bug.

@PVince81
Copy link
Contributor Author

@schiesbn is there any situation you can think of when the file id might change if encryption is enabled ?

@PVince81
Copy link
Contributor Author

Partial fix here: #13385
But still need to take care of folders.

@PVince81
Copy link
Contributor Author

There is no good short term approach for 8.0.

Hopefully we can try finding an approach similar to cross storage file renames: #13360 from @icewind1991

I also thought about using rename hooks for the shares and tags, but then it means gathering the list of all files to be renamed in the "pre-rename" hook, then in "post-rename" find out whether the file ids changed, and if yes, update the share and tags tables. But this solution would not solve the case for the sync client anyway and would be have to be thrown away once we implement a proper file-id preserving solution.

So, leaving this scheduled to 8.1.

@icewind1991 let's discuss the best solution in the coming weeks.

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 5, 2015

Yet another situation that can break file ids: whenever a file was moved directly on an external storage. The file scanner might not be able to detect that the file was moved and believe that it is a new file, so it gets an new file id.
Same happens if "files:scan" is run whenever the filecache doesn't match the real storage, moved files will get a new file id. The files that stayed should keep the file id though.

@dragotin
Copy link
Contributor

dragotin commented Feb 5, 2015

Reason for not detected moves in the clients.

@guruz
Copy link
Contributor

guruz commented Mar 23, 2015

Why do you guys not use the OS/storage specific identifiers for both FileID and ETags?
For example for the FileID you use the inode+some_volume_fs_specific_unique_identifier (only as initial value)
For ETag you use FileID+mtime+size (on each change)

Just because FileID and ETag are opaque strings does not mean you can not "seed" them with relevant values.

CC @ogoffart

@PVince81
Copy link
Contributor Author

Maybe because not all external storages would support it (ex: SMB)
Also I'm not sure how it works when using "object store" as main storage.

@icewind1991 tried an alternative approach which stores the file id in extended attributes: #14546

@guruz
Copy link
Contributor

guruz commented Mar 23, 2015

Maybe because not all external storages would support it (ex: SMB)

(which could fall back to the codepath that creates a random/incremental one)

Also I'm not sure how it works when using "object store" as main storage.

I'd guess every object storage has some way of representing this by IDs or revision numbers or whatever

@PVince81
Copy link
Contributor Author

So far we seem to have shied away from any solution that would be "local storage specific" or bad/slow/impossible for external storages. Maybe it's time to change ?

@icewind1991
Copy link
Contributor

file id's are not determined by the storage backends since they need to be global (and because they are numeric prefixing them isn't trival)

@MorrisJobke
Copy link
Contributor

More broken shares are there: #14084

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 9, 2015

Good news! @icewind1991 has brought a PR that keeps file ids between storages (that includes shared folders): #15360

I also checked master and deleting a file to trashbin then restoring will also keep the file id. 😄

@PVince81
Copy link
Contributor Author

Thanks to #15360 most of the fileid issues are resolved.
I'll close this ticket but we need to follow up on the remaining one, which is related to remote file changes on external storages: #11533

@PVince81
Copy link
Contributor Author

Please note that when moving files between external storages (or when deleting from external storage), the fileid is also kept properly, which is nice. (I just tried this and it works 😄)

The outstanding issue is only when files are changed remotely (outside of ownCloud).

@moscicki
Copy link

moscicki commented May 8, 2015

"Stable file id" is broken by design for syncing as mentioned in owncloud/client#2725 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants