ImageLoader has a couple of issues I'm going to discuss now.
First of all, let's consider that the only code that still uses ImageLoader is the SLDStyleFactory, which uses it to load external graphics in non interactive mode. At the current state, there is no code trying to use that class in interactive mode.
The responsibilities for ImageLoader are:
- load external images, eventually returning null immediatly but keep on loading the file in the background (interactive mode)
- load external images blocking the caller (non interactive mode)
- cache the results so that the image loading does not occurr over and over again
Image loading is done using MediaTracker, a bunch of sleep(xxx) and a 10 seconds timeout which is probably the cause
of the issues we're seeing in GeoServer (see linked issue).
If we give up interactive loading (which is not used), we could load an image by simply using ImageIO.read(url), that would
be simpler and possibly more reliable.
Image caching is to be kept, but current code has two limitations:
- image cache is static and unbounded, meaning that once an image is loaded, it'll stay there as long as the application lives. This may be troublesome, especially if someone loads by mistake a big image instead of small icons. Using a SoftValueHashMap should address this problem.
- the cache is keyed by url, no attempt is made to see if the file system or the external server has a fresher copy of the data (maybe the content has been replaced). We should avoid that and make sure the url does point to the same data (e.g., if it's a file use latest modification date, if it's a http connection consider caching limits or do a conditional get retrieving data only if it has been modified after a certain date...
not sure there is any general way to do so, for http see http://www.oreillynet.com/onjava/blog/2004/07/optimizing_http_downloads_in_j.html)
I would like your opinions on this one. Am I missing something? Any suggestions on how to proceed?