PagefilesManager should tolerate wrong datetime of files

[gerritvanaaken]

Short description of the issue

$page->filesManager->getFile('some_image.jpg')

If the database entry for "some_image.jpg" has a created-datetime like "1970-01-01 01:00:10", it does not pass the validation and won’t get selected, even if the image itself would be perfectly okay to handle.

Expected behavior

Image gets selected and can be used as an image object.

Actual behavior

Return is NULL

Steps to reproduce the issue

1. Upload an image within CK editor (see image upload via CK Edit popup –> wrong datetime in database #41)
2. in the template, try $page->filesManager->getFile('some_image.jpg')
3. You get NULL as a result

Setup/Environment

• ProcessWire version: 3.0.35
• (Optional) PHP version: 5.6.25
• (Optional) MySQL version: 5.6.28

[ryancramerdesign]

That timestamp indicates the file is in a temporary state between being uploaded and the page being saved. Meaning, it's not yet been confirmed that the file will actually remain. Thus it isn't technically part of the page yet, so the return value is correct. Are you seeing this occur after the page has already been saved (and image is part of it)?

[teppokoivula]

Coincidentally I'm currently debugging an issue related to "disappearing" images. I've found at least one situation where created date is not updated after a page has been saved, but still got some work to do before it can be reduced to a proper test case.

This is pretty vague, but in my case there's a module that hooks into Pages::saveReady and, among other things, clones current page and saves the clone. I'm not sure if this is the specific part that breaks things, but it does seem suspicious :)

Edit: confirmed that hooking into Pages::saveReady and cloning the page being saved is, at least in this particular case, the culprit. It prevents the created date from being updated, and thus it remains as 1970-01-01 02:00:10 (I'm assuming that the 1h difference related to timezone or DST).

[ryancramerdesign]

Files are made temporary by uploading them in the page editor via ajax, and made permanent by hitting "save" in the page editor. I have a feeling your clone was made during the ajax upload request, as the files would be temporary at that point. If the user uploads files and never saves the page, the files remain temporary and are deleted the next time they try to edit it. If the user instead saves the page, then the files become permanent. But at that point, presumably your clone has already been made. Since your clone is not actually being edited in the admin, there's no user to click "save" to confirm those files should be made permanent. If the user were to go and edit the clone at some point, the temporary files would be automatically deleted. One potential solution might be to avoid cloning in a saveReady() when files are being uploaded, i.e.

if($config->ajax && isset($_SERVER['HTTP_X_FILENAME'])) {
  // no clone
} else {
  // clone
}

[teppokoivula]

Ryan, I may be misunderstanding your comment, but just to clarify things and provide a bit more context, this is what happens in my case:

• After I upload an image, it shows up just fine. The file is created and in database table field_image there's a row for it with created set to 1970-01-01 02:00:10.
• After checking the database value (just to be sure) I return to browser and toggle a checkbox (a Page field; this tells the system when to trigger the cloning part and also where the clone should be placed)
• After hitting save, the clone gets created by a method hooked into Pages::saveReady, Page field triggering the cloning process is cleared, and the created date for previously uploaded image remains as-is (it is not updated to reflect actual date and time).

If I comment out this line in my hook method, everything works as expected again:

$duplicate = wire('pages')->clone($page, $parent_page);

It's probably also worth mentioning that the clone doesn't have this issue: the created date for the image in the cloned page matches current date. It's just the original page that has this issue :)

---

Here's a simplified test case. After pasting this to /site/templates/admin.php I'm seeing the behaviour explained above on another site:

$wire->pages->addHookAfter('Pages::saveReady', function(HookEvent $event) {
    $page = $event->arguments[0];
    if ($page->id != 5483) return; // avoid an infinite loop..
    if (wire('config')->ajax && isset($_SERVER['HTTP_X_FILENAME'])) {
        // no clone
        wire('session')->message("Didn't create a clone");
    } else {
        $clone = wire('pages')->clone($page, $page->parent);
        wire('session')->message("Clone created: $clone->url");
    }
});

While testing I also noticed another quirk: with this code in place I can't remove images either. Everything else saves as expected, but it seems that no changes to images will stick. Is there something I'm missing here? :)

I have no idea if this is directly related to the issue posted by @gerritvanaaken. It's possible that the only connection is that there really seem to be some situations under which the temporary created value becomes permanent.

Please let me know if you think that this should be discussed in it's own issue!

[gerritvanaaken]

As mentined in my new comment to issue #41, the conversion from temporary to permanent only will be performed if you save the page which physically contains the image. If the image was being ajax-uploaded to a different page, the save event will never be applied on the correct page.

[teppokoivula]

Looks like there are indeed multiple ways to reproduce this issue. In my case I switched the hook to Pages::saved, and that works just fine. I still think it's unexpected behaviour that performing a clone operation would mess up images altogether.

[ryancramerdesign]

As mentined in my new comment to issue #41, the conversion from temporary to permanent only will be performed if you save the page which physically contains the image. If the image was being ajax-uploaded to a different page, the save event will never be applied on the correct page.

This is what I missed before. I was able to reproduce it when uploading to a different page than the one being edited. This has been fixed. Thanks.

Looks like there are indeed multiple ways to reproduce this issue. In my case I switched the hook to Pages::saved, and that works just fine. I still think it's unexpected behaviour that performing a clone operation would mess up images altogether.

If I understood correctly, the clone was being attempted on a page with changes, before those changes were saved to the DB. Given that, I don't think saveReady() would be a safe place to clone a page, because the page in the DB and file system is different from the page in memory. If you can accomplish what you need with the saved() hook instead, that should be safe.

[teppokoivula]

@ryancramerdesign For the time being I'm happy with the workaround mentioned above.

Mainly mentioned the issue in case it's a sign of something not working as expected: feels strange that cloning a page would break anything, let alone images, on the source page, which (assuming that I understand what "cloning" means in this context) shouldn't be affected in any way.

It would've made some sense if it broke things on the cloned page, but that one works just fine.

Anyway, if you think that this is indeed correct behaviour and not a sign of a potential issue, that's good enough for me :)