fix error if the target destination is not writeable

[rommelfreddy]

Hey @jissereitsma

on deployments where the most folders are non-writeable folders the module will break with a Permission denied error. This happens explicit on all pub/static folders.

This fix will add a cache-folder to the destination URL, if the destination folder/file is not writeable.

Maybe this will be the base of a new option in the future, to make it possible to save all optimized images in a cache folder, to separate them from the original source.

[rommelfreddy]

Hey @jissereitsma could you tell me, if this MR got merged anytime, and yes, do you have an idea when it could be.

[jissereitsma]

Even though the PR is quite small, I'm a little bit hesitant of accepting it too quickly. First of all, I've been seeing many PRs coming in lately, while the usage of the extension is also spiking (hurray!). But this also increases the importance of not breaking anything. I'm intending to create various tests (unit tests, integration tests) to safeguard the current behaviour and to make sure the PR is not breaking anything. The current PR passes unit tests, but that's because those unit tests are far too limiting (my bad).

Second, I think the code that is responsible for retrieving the new WebP path for a specified JPG path should be moved to a separate class (for testing purpose, but also for readability), so that the responsibility for creating the new path is different from the responsibility of actually converting the image.

Currently, I have a backlog due to some personal circumstances but I aim to get the mentioned work done in the next week or so.

[jissereitsma]

As mentioned a major rewrite was scheduled that would put this PR in a totally new perspective (and the code definitely conflicts). Version 0.3.0 of NextGenImages is now out with support for a separate media folder media/nextgenimages/. Would that already fix your issue as well? Or would the folder really need to be something specific (so configurable)?

[jissereitsma]

Sorry for all the hard work that you put into this (and I'm really grateful for this), but I think this PR is outdated with the new feature I mentioned earlier. If one image succeeds to be written and another fails, then there is something horribly wrong with the filesystem permissions. However, if all writes are failing simply because the underlying folder is not writable, the new feature in this module allows to use a "caching folder" instead. With the latest 0.3.16 release the location of this folder is also made configurable.

[rommelfreddy]

yes. this is outdated.