[RFC][Icedtea-web]: Changed how cache works.

[Andrew Su]

----- Original Message -----
> From: "Omair Majid" <omajid at redhat.com>
> To: "Andrew Su" <asu at redhat.com>
> Cc: "Denis Lila" <dlila at redhat.com>, "IcedTea" <distro-pkg-dev at openjdk.java.net>
> Sent: Monday, April 18, 2011 10:20:28 AM
> Subject: Re: [RFC][Icedtea-web]: Changed how cache works.
> On 04/16/2011 07:39 PM, Andrew Su wrote:
> >
> >
> >>>
> >>>>> + public synchronized boolean addEntry(String identifier,
> >>>>> String path) {
> >>>
> >>>>> + public synchronized boolean removeEntry(String key) {
> >>>
> >>>>> + public synchronized boolean updateEntry(String oldKey) {
> >>>
> >>> I would have been happier with all these methods taking
> >>> identifiers or keys. As such, there is on way to add an entry and
> >>> then remove it (as you wouldnt know the key for it).
> > I've changed it to take a key for addEntry instead of identifier.
> 
> I dont think this is the right way. Now the implementation knowledge
> is
> spread across multiple classes. If you change the key in
> makeNewCacheFile, CacheLRUWrapper will need need to be updated too :(
> Perhaps a fix would be to create a method like generateKey(Sting path)
> in CacheLRUWrapper? The key returned by this method would be used in
> addEntry.

I've added generateKey to CacheLRUWrapper.

> 
> >>>
> >>> Have you tested this while offline? I am afraid that this might
> >>> break running jnlp applications offline.
> > If by testing this offline you mean run it using a jnlp file, let it
> > cache, then run jnlp file while disconnected from the internet and
> > still be able to launch it? Then yes, however it fails to launch the
> > cached item. This actually happens with HEAD without my patch too.
> > (Running jnlp file while connected works)
> >
> 
> Ugh. That sucks :( I know this was working in Feburary (my demos at
> FOSDEM were running offline). I wonder if I accidentally broke it.

It's ok, nothing we can't fix ;)

> 
> >>>
> >>> Everything else looks fine to me.
> >>>
> > I've made some minor changes, notably adding an entry for compressed
> > files, since when files are compressed, comparison with extracted
> > file's size against the recorded size will definitely be different
> > (so I avoid this check, have let isCurrent handle the rest by
> > checking last-modified).
> >
> 
> Are the markCompressed/isCompressed methods used anywhere? If not,
> could
> you please make that a separate patch?

Yes, I've added the use of isCompressed in CacheEntry.isCached().
Reason being that if the item was extracted, the content-length reported
by the connection would differ from actual file size, thus forcing it to
download without actually checking the modified time.

> 
> > Thanks for reviewing my patches!
> >
> 
> No problem!
> 

Is the patch okay now?

Cheers,
  Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20110418_cache_vXX.patch
Type: text/x-patch
Size: 32347 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110418/954dd3dc/20110418_cache_vXX.patch