Bug #796
closedDo not cache Images if image is not exist
Added by Eric Hou over 14 years ago. Updated about 14 years ago.
0%
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
Updated by Eric Hou over 14 years ago
- Status changed from New to Assigned
- Assignee set to Eric Hou
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.
Updated by Eric Hou over 14 years ago
- Assignee changed from Eric Hou to Amber Herold
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.
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
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.
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
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?
Updated by Amber Herold over 14 years ago
- Assignee changed from Amber Herold to Eric Hou
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
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();
}
Updated by Amber Herold over 14 years ago
- Status changed from Assigned to In Test
Updated by Amber Herold about 14 years ago
- Target version set to Appion/Leginon 2.1.0
Updated by Amber Herold about 14 years ago
The tests work on fly with my workspace.
Updated by Amber Herold about 14 years ago
- Status changed from In Test to Closed
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
Updated by Eric Hou about 14 years ago
- Status changed from Assigned to Closed
it works on new install.