[rfc][icedtea-web] Allow cache directory to be changed during runtime

[Jiri Vanek]

On 01/23/2015 08:09 PM, Jie Kang wrote:
>
>
> ----- Original Message -----
>> On 01/15/2015 07:16 PM, Andrew Azores wrote:
>>> On 01/05/2015 02:49 PM, Jie Kang wrote:
>>>> Hello,
>>>>
>>>> This patch consolidates the cacheDir instance to a single place and allows
>>>> us to change it during
>>>> run-time. It also modifies a unit test to use this new feature. This
>>>> feature is aimed mainly at
>>>> unit tests that need the cache, but do not want to modify the
>>>> icedtea-web's user cache.
>>>>
>>>> Thoughts? Okay to push?
>>
>> I'm wondering why it is needed at all.
>>
>> JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_USER_CACHE_DIR);
>> is the place
>> where you can set the dir.
>>
>>
>> For unittest you may force refelction to do the job. if the chnage of the
>> property isnot enough.
>>
>>
>> What I would rather see is, to get rid of all the static stuff in
>> CacheLRUWrapper and have
>> possibility to create in instance.
>> This instance will be also much more testable. But maybe it is overkill.
>
> Hello,
>
> Yes this is mainly to address that for unit tests, JNLPRuntime.[...] doesn't work since CacheLRUWrapper and CacheUtil both use static variables that get initialized on first use of either class. I think getting rid of all the static stuff in LRUWrapper and making it instantiable is a great idea. I see this patch as a baby-step towards doing that.
>
> I very strongly prefer NOT using reflection to set the cacheDir value in the unit tests.
>
> Also, please see [1] for a longer explanation
>
> [1] http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-January/030470.html
>
>
  Yes. But still - if do this, do it properly, and so go with :

" to get rid of all the static stuff in
  CacheLRUWrapper and have
  possibility to create in instance.
  This instance will be also much more testable"

and so go for singleton instead of static.


J.


>
>
>>
>> J.
>>>>
>>>>
>>>> Regards,
>>>>
>>>
>>> I think this looks generally ok. Maybe as a separate enhancement changeset,
>>> but since you're working
>>> in here anyway, could you look at doing some refactoring to store the cache
>>> directory as something
>>> better than a String? As a File, or using Java 7's nio2 stuff, a Path
>>> maybe?
>>
>> On one side - yes, on the other - why? What operations are mainly done on the
>> "string" string ops or
>> file ops - I have not checked code, but seems like concate is most often :)
>>
>> But yes - it is directory, so file or path is probably better then string.
>> Please, do this refactoring as separate changset anyway.
>>
>> J.
>>
>