[RFC][Icedtea-web] Add cache viewer for control panel.

[Andrew Su]

----- "Omair Majid" <omajid at redhat.com> wrote:

> From: "Omair Majid" <omajid at redhat.com>
> To: "Andrew Su" <asu at redhat.com>
> Cc: "OpenJDK" <distro-pkg-dev at openjdk.java.net>
> Sent: Tuesday, December 14, 2010 1:41:12 PM GMT -05:00 US/Canada Eastern
> Subject: Re: [RFC][Icedtea-web] Add cache viewer for control panel.
>
> On 12/13/2010 05:08 PM, Andrew Su wrote:
> > Hello,
> >
> > I have attached a patch which adds a cache viewer for looking at
> the
> > cache directory (specified by deployments.properties).
> >
> > This allows the user to view and delete the contents in the cache
> folder.
> > It displays the name of the file, the path to the file, the protocol
> the
> > file was obtained from, the domain which this file came from, file
> size
> > and date last modified.
> >
> > Comments?
> >
> 
> Overall, it looks fine. But I do have a few suggestions on how it can
> be 
> improved:
> 
> - Please update the patch so it builds with current HEAD. All that is
> 
> required is changing the imports from
> import net.sourceforge.jnlp.runtime.DeploymentConfiguration;
> to
> import net.sourceforge.jnlp.config.DeploymentConfiguration;
> 
> - Please make the cache viewer window resizable. It's quite hard to
> see 
> the full path and the full domain without changing the window size.
Will change these two.

> 
> - The size column doesn't specify the units of the size (is that
> bytes?)
Whoops.

> 
> - I would like to be able to sort by columns.
Okay, I'll add to updated patch.

> 
> - Some javadoc comments seem to diverge from the code style guidelines
> 
> [1] without good reason. Please fix that.
This was unintentional, ctrl+shift+f in eclipse using the settings file from tip still made it line wrap for me, will fix manually.

> 
> - Is there a reason you use Vectors rather than ArrayLists? Vectors 
> synchronize all of their operations, which is going to be slower.
Before posting I had it generate the data in add components then passed in the data and column to the instructor for JTable to populate, it doesn't accept ArrayList. I will change it to use ArrayList for the updated patch.

> 
> - Can you split out methods that traverse the cache from this cache 
> viewer? It might be more appropriate to create a new class in 
> net.sourceforge.jnlp.cache for this.
Sure thing!

Thanks for looking it over!

Regards,
  Andrew

> 
> Thanks,
> Omair
> 
> [1] http://icedtea.classpath.org/wiki/IcedTea-Web#Code_style