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

Thumbnails not regenerated when reverting to an earlier version of a file #10559

Closed
jnfrmarks opened this issue Aug 20, 2014 · 16 comments · Fixed by #15098
Closed

Thumbnails not regenerated when reverting to an earlier version of a file #10559

jnfrmarks opened this issue Aug 20, 2014 · 16 comments · Fixed by #15098

Comments

@jnfrmarks
Copy link

Steps to reproduce

  1. Make sure the Versions app is enabled
  2. Create a new document (click on "new"->text file from the "files" view)
  3. Enter some basic text; save; close
  4. Validate that the thumbnail next to the file is accurate
  5. Edit the file a second time
  6. Validate that the thumbnail changed as expected.
  7. Highlight the file, select "versions", select previous version
  8. The thumbnail did not revert

I see this with both FF and chrome. I cleared the cache to ensure that this isn't a caching issue.

p1

p2

Expected behaviour

The thumbnail should revert too

Actual behaviour

The thumbnail reflects the earlier version

Server configuration

ownCloud version: (see ownCloud admin page)

ownCloud 7.0.1 (daily) Build:2014-08-20T03:18:51+00:00

@karlitschek
Copy link
Contributor

@schiesbn @georgehrke

@georgehrke
Copy link
Contributor

Looks like a missing hook

@MTRichards MTRichards added this to the 2014-sprint-04-next milestone Sep 5, 2014
@MTRichards
Copy link
Contributor

Would be good to regenerat thumbnails when a file is reverted, and also when a new file is uploaded with the same name. See #10822
Related?

@georgehrke
Copy link
Contributor

@MTRichards

Just like in #10521:
#10822 is related to the gallery app. This is an bug in the preview system itself

@georgehrke georgehrke self-assigned this Sep 5, 2014
@MTRichards
Copy link
Contributor

Thanks @georgehrke

@georgehrke
Copy link
Contributor

Ok, so I digged into this problem and it seems like some logic error / missing hook in the versions app.
When you create a new file it gets an Id.
Let's say the file is called test.txt and the Id is 1.
After editing test.txt:

  • the file is still called test.txt
  • the id of username/files/test.txt is still 1
  • a copy of the old version of username/files/test.txt was created
    • it's called username/files_versions/test.txt.{revisionTimeStamp}
    • its id is 2
  • a write hook is emitted -> the old preview is deleted

When reverting to an old version:

  • the file is still called test.txt
  • the id of username/files/test.txt is still 1
  • a copy of the new version (that's being overwritten by the old version) was created
    • it's called username/files_versions/test.txt.{revisionTimeStamp}
    • its id is 3
  • no write hook is emitted -> the old preview is not deleted
    • Neither an \OCP\Versions hook nor some OC_Filesystem delete/write hooks

When grepping over the files_versions code I noticed that there are actually no hooks executed when reverting to an old file. There are only hooks being emitted when the file is deleted because a user's disk space almost full.

@schiesbn I am probably missing something here. How are other apps supposed to know when a file was reverted to an old version?

@craigpg craigpg modified the milestones: 2014-sprint-05-next, 2014-sprint-04-current Sep 15, 2014
@georgehrke
Copy link
Contributor

ping @schiesbn

@craigpg craigpg modified the milestones: 2014-sprint-06-next, 2014-sprint-05-current Sep 29, 2014
@georgehrke
Copy link
Contributor

ping @schiesbn

@craigpg craigpg modified the milestones: 2014-sprint-07-next, 2014-sprint-06-current Oct 12, 2014
@georgehrke
Copy link
Contributor

ping @schiesbn

@craigpg craigpg modified the milestones: 2014-sprint-08-next, 2014-sprint-07-current Oct 27, 2014
@georgehrke
Copy link
Contributor

@schiesbn please comment on #10559 (comment)

@craigpg craigpg modified the milestones: 2014-sprint-09-next, 2014-sprint-08-current Nov 10, 2014
@oparoz
Copy link
Contributor

oparoz commented Jan 22, 2015

When reverting to an old version: no write hook is emitted -> the old preview is not deleted

@schiesbn @PVince81 Has the missing hook been added to OC8?

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.1-next, 2014-sprint-09-next Jan 22, 2015
@schiessle
Copy link
Contributor

@georgehrke I think you are right. We didn't noticed this until now because no other app had to listen to a revert event.

OC\Filesystem doesn't trigger a hook on revert because to operation happens outside of data/<user>/files. This means that we most likely need a versions related hook. I would suggest to just emit a hook here https://github.com/owncloud/core/blob/master/apps/files_versions/lib/storage.php#L273 with all the information you need.

This could be also interesting for the activity app cc @nickvergessen

@schiessle
Copy link
Contributor

BTW, sorry that it took me so long to response to this issue. Feel free to ping me on IRC, Jabber or E-Mail the next time you need a answer and I don't reply in time to a github issue.

@nickvergessen
Copy link
Contributor

Well activity app is unaffected, since it always displayes the current preview.

@SergioBertolinSG
Copy link
Contributor

Please note, not only the thumbnail but the big previewed picture.

@SergioBertolinSG
Copy link
Contributor

Fixed in last master.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.