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

[BUG] When regenerating derivatives, the Media information (date edited) is not altered. #1050

Open
rosiel opened this issue Aug 8, 2024 · 2 comments

Comments

@rosiel
Copy link
Member

rosiel commented Aug 8, 2024

When we create derivatives that overwrite existing derivatives, the user can't see anything in the interface to signify that there was any change.

Currently, to see if a derivative worked, we have to physically inspect the file (often with Developer Tools' Network Tab open so that "Disable Cache" is active!)

The code that writes the file directly is here:

public function updateSourceField(
MediaInterface $media,
$resource,
$mimetype
) {
$source_field = $this->getSourceFieldName($media->bundle());
$file = $this->getSourceFile($media);
// Update it.
$this->updateFile($file, $resource, $mimetype);
$file->save();
// Set fields provided by type plugin and mapped in bundle configuration
// for the media.
foreach ($media->bundle->entity->getFieldMap() as $source => $destination) {
if ($media->hasField($destination) && $value = $media->getSource()->getMetadata($media, $source)) {
$media->set($destination, $value);
}
// Ensure width and height are updated on File reference when it's an
// image. Otherwise you run into scaling problems when updating images
// with different sizes.
if ($source == 'width' || $source == 'height') {
$media->get($source_field)->first()->set($source, $value);
}
}
$media->save();
}
/**
* Updates a File's binary contents on disk.
*
* @param \Drupal\file\FileInterface $file
* File to update.
* @param resource $resource
* Stream holding the new contents.
* @param string $mimetype
* Mimetype of new contents.
*/
protected function updateFile(FileInterface $file, $resource, $mimetype = NULL) {
$uri = $file->getFileUri();
$destination = fopen($uri, 'wb');
if (!$destination) {
throw new HttpException(500, "File $uri could not be opened to write.");
}
$content_length = stream_copy_to_stream($resource, $destination);
fclose($destination);
if ($content_length === FALSE) {
throw new HttpException(500, "Request body could not be copied to $uri");
}
if ($content_length === 0) {
// Clean up the newly created, empty file.
unlink($uri);
throw new HttpException(400, "No bytes were copied to $uri");
}
if (!empty($mimetype)) {
$file->setMimeType($mimetype);
}
// Flush the image cache for the image so thumbnails get regenerated.
image_path_flush($uri);
}

I propose that we
a) update the last updated date on the Media when this happens [is this a thing we can do?]
b) or, consider changing this so that it does what a normal drupal UI file replace does - leaves the original file in place (as it is referenced from older revisions of the Media) and creates a new file for the new file. Drawback: The URL of the file will change. Unless you're using a method to rewrite the URL that the file appears at, such as https://www.drupal.org/project/media_entity_file_redirect .

What steps does it take to reproduce the issue?

  • When does this issue occur?

  • Which page does it occur on?

  • What happens?

  • To whom does it occur (anonymous visitor, editor, administrator)?

  • What did you expect to happen?

Which version of Islandora are you using?

Any related open or closed issues to this bug report?

Screenshots:

@adam-vessey
Copy link

Personally, I lean towards "b)", with creating new media revisions. I don't consider the URL changing as a drawback, as by virtue of pointing at a different URL, some issues with caching go away. Hotlinking to specific files from external systems shouldn't typically be recommended.

That said, if going such a route, we might want to look at introducing some kind of "garbage collection" mechanism to (help) delete old(/non-current) revisions and the files to which they refer? Some kind of life-cycle definition around them?

@rosiel
Copy link
Member Author

rosiel commented Aug 14, 2024

Shaw, S. (2014), during the call: change the media height/width when regenerating.

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

No branches or pull requests

2 participants