Project

General

Profile

Actions
Copy link

Bug #796

closed

Do not cache Images if image is not exist

Added by Eric Hou over 14 years ago. Updated about 14 years ago.

Status:
Closed
Priority:
Urgent
Assignee:
Eric Hou
Category:
-
Target version:
Appion/Leginon 2.1.0
Start date:
07/29/2010
Due date:
% Done:

0%

Estimated time:
Affected Version:
Appion/Leginon 2.0.2
Show in known bugs:
No
Workaround:

Description

The cache system will still caching images even through the images are not in the file system, we need to stop this happening.

Thanks.

Eric

Actions
Copy link
 #1

Updated by Eric Hou over 14 years ago

  • Status changed from New to Assigned
  • Assignee set to Eric Hou
Actions
Copy link
 #2

Updated by Eric Hou over 14 years ago

  • Status changed from Assigned to In Code Review
  • % Done changed from 0 to 100

After the code changed, the caching system will not cache the image if source (mrc file) is not there. Originally the caching system will create a blank jpg file if it can not find the source (mrc file).

Need detail testing on this one, because I am not sure it work right.

To test:
Delete the cached folder for the session you want to test.
Go to image viewer, and view that session.
Check is all the images got cached??

Then Delete the cached folder for that session.
move the source mrc image to somewhere else.
Go to the image viewer and try to view those image for that session.
Image viewer should display empty image.
Then put the source images back.
And go back to image viewer.
This should display the images again and also cached them.

Thanks.

Actions
Copy link
 #3

Updated by Eric Hou over 14 years ago

  • Assignee changed from Eric Hou to Amber Herold
Actions
Copy link
 #4

Updated by Amber Herold over 14 years ago

  • Assignee changed from Amber Herold to Eric Hou

source:trunk/myamiweb/getparentimgtarget.php#L118
source:trunk/myamiweb/imagehistogram.php#L60

Eric, did you consider what happens with the other calls to getImage() if false is returned rather than a blank image? It is not obvious to me that everything will function without errors.

Actions
Copy link
 #5

Updated by Eric Hou over 14 years ago

  • Assignee changed from Eric Hou to Amber Herold

Not so sure.
Maybe you can write an test case to see what happen.

Eric

Actions
Copy link
 #6

Updated by Amber Herold over 14 years ago

  • Assignee changed from Amber Herold to Eric Hou

This seems like pretty fragile code. What about throwing an exception if the image does not exist? Then in the catch, you can continue. For the other instances of the call you can set the image to a blank image in the catch so you do not change the current behavior.

Actions
Copy link
 #7

Updated by Eric Hou over 14 years ago

  • Assignee changed from Eric Hou to Amber Herold

Can you write an example to with the current system and make sure the Javascript part will display the exception, and please don't make a blank image in the cache folder if the image is not there. This is the whole point of this bug.

Thanks.

Eric

Actions
Copy link
 #8

Updated by Amber Herold over 14 years ago

What would you want to display? I don't think it is necessary to display anything. You would just throw an exception in the call to getImage. When the exception is cought, you execute a continue which would skip the rest of the loop, and therefore not add an image to the cache. No?

Actions
Copy link
 #9

Updated by Amber Herold over 14 years ago

  • Assignee changed from Amber Herold to Eric Hou
Actions
Copy link
 #10

Updated by Eric Hou over 14 years ago

  • Status changed from In Code Review to Assigned
  • Assignee changed from Eric Hou to Amber Herold
  • % Done changed from 100 to 0

Amber has better solution for this situation.
Assign this bug to Amber.

Thanks.

Eric

Actions
Copy link
 #11

Updated by Amber Herold over 14 years ago

I'm syncing my workspace but am not checking in these changes b/c it sounds like Jim prefers not to have a blank image for the other calls. I think these need to be very well tested however.

I'm leaving the code here that would be used to add exception handling, just in case we decide to use it later.

This is the code for image.inc line 402

    } else {
        //$img = blankimage();
        //instead to create a blankimge(). We need to return false.
        //return false;

                // Instead of returning false, throw an exception.
        throw new Exception("In getImage(), the requested image does not exist.");
    }
    return $img;

makeJPEG.php line 131
try  
{
    $img = getImage($sessionId, $id, "", $params);
}
catch(Exception $e)
{
    // when there is no image. skip it....
    continue;
}

imagehistogram.php line 60:
    try {
        $img = getImage($sessionId, $imgId, $preset, $params);
    }
    catch(Exception $e) {
                //could not find an image so set it to a blank image
        $img = blankimage();
    }

getparentimagetarget.php line 118
        try {
            $img = getImage($sessionId, $id, $preset, $params);
        }
        catch(Exception $e) {
              //echo 'Message: ' .$e->getMessage();
              $img = blankimage();
        }

Actions
Copy link
 #12

Updated by Amber Herold over 14 years ago

  • Status changed from Assigned to In Test
Actions
Copy link
 #13

Updated by Amber Herold over 14 years ago

  • Target version set to Appion/Leginon 2.1.0
Actions
Copy link
 #14

Updated by Amber Herold about 14 years ago

The tests work on fly with my workspace.

Actions
Copy link
 #15

Updated by Amber Herold about 14 years ago

  • Status changed from In Test to Closed
Actions
Copy link
 #16

Updated by Eric Hou about 14 years ago

  • Status changed from Closed to Assigned
  • Assignee changed from Amber Herold to Eric Hou
  • Priority changed from Normal to Urgent

I need to double check this. I don't think it works like it should be.

Thanks.

Eric

Actions
Copy link
 #17

Updated by Eric Hou about 14 years ago

  • Status changed from Assigned to Closed

it works on new install.

Actions
Copy link

Also available in: Atom PDF