[rfc] [icedtea-web] make PAthsAndFilles to follow seted paths

Jiri Vanek jvanek at redhat.com
Wed Apr 1 16:24:41 UTC 2015


On 03/30/2015 02:52 PM, Jiri Vanek wrote:
> On 03/27/2015 09:01 PM, Jie Kang wrote:
>>
>>
>> ----- Original Message -----
>>> Hi!
>>>
>>> This is first of at least three pathces tro make it work
>>> this one - i adding difference between getFile/Path and
>>> getDefaultFile/DefaultPAth + adapting few
>>> (the most important) PathsAndFiles
>>> So this one is especially about concept. There rised issue, that
>>> PathsandFiles become depnding on
>>> JnlpRuntime.getConfig() - the result of it is later-lazy loading of
>>> RECENTLY_USED_FILE.
>>> Also note the behaviour of deployment.user.security.policy
>>>
>>> Second part of this work will be focused on adapting missing "adjustable
>>> files"
>>>    deployment.user.security.trusted.cacerts
>>> deployment.user.security.trusted.certs
>>> deployment.user.security.trusted.clientauthcerts
>>> deployment.user.security.trusted.jssecacerts
>>> deployment.user.security.trusted.jssecerts
>>>
>>> And ensure, that those are never ever used in itw - only the PathsAndFiles
>>>
>>> ThirdPart will be then focused on being sure, no dangling insatnces are
>>> around - taht everything is
>>> using PAthsAndFiles directly, and so runtime change to any
>>> InfrastructureFileDescriptor will take
>>> imidiate effect. (aka changing cache in runtime durng testing)
>>>
>>> But this this third part is far faraway...
>>>
>>> BAck to this patch:
>>>    I let Defaults.java use CACHE_DIR.getDefaultFullPath() instead of
>>>    CACHE_DIR.getFullPath(). Tahts
>>> actually very correct - because those are default values. Then I have added
>>> possibility to override
>>> getFullPath by get from properties + few small changes for docs generation.
>>>
>>> Thoughts?
>>
>> Hello,
>>
>> Nice! Review below:
>>
>>
>> +     this(PathsAndFiles.getRECENTLY_USED_FILE().getFile(), PathsAndFiles.CACHE_DIR.getFile());
>>
>> Can you explain a little more on why there is a getter for recently used file. Where is the later-lazy loading and what problem is occurring that makes it so you need to have a getter?
>
> It is using  PathsAndFiles.CACHE_DIR.getFile() for its creation.
> It is based on property value
> So in this moment the jnlpconfig have to exists, that means that deployment.properties must be read.
> So laxy loading menas as late as possible. Loading it in static phase, will create pretty nice initialization error.
>>
>> Also please don't capitalize words in the function:
>> s/getRECENTLY_USED_FILE/getRecentlyUsedFile
>>
>>
> will be done
>> +            return gcpd(DeploymentConfiguration.KEY_USER_CACHE_DIR);
>>
>> Please give the function gcpd(...) a descriptive name that gives me an idea of what it's doing without having to look at the function's implementation.
>
> Itis just shortuct method getConfgiPropertyDescriptor.
>
> I think it will be removed in future iterations of this. I'm now working on PArt three, and it is already gone...
>
>>
>> +        //its not recomanded to override default locations methods
>>
>> s/recomanded/recommended
>
> will be done.
>>
>>
>> +        public String getDefaultFullPath() {
>>               return clean(systemPathStub + File.separator + pathStub + File.separator + fileName);
>>
>> For the function clean(...), can you please either rename it to be more descriptive or add comments to the function. Just seeing the function name I have very little clue as to what it does.
>
> Javadoc will be added.
>>
>> +        private String getStub() {
>>
>> Can you add comments to this? I don't know what you mean by 'stub'.
>
> I can't? :)
>
> Sure. I will write javadoc.  It is method, which is providing the nice variable-like outputs to docs.
>>
>> Other parts look okay, but please make sure you test this thoroughly. Are there tests for this?
>>
>
> Hard to test in this time. I did as throughlky as I could. I think it is good approach to follow.
> With part two done (already on review) it is much more testable, and seems to be working fine.

I will add much more tests during parts II-IV
>
>
> Thanx for check!
>

Here you go. Jsut moinors as you insisted.

Ok for head?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: makePathsAndFilesObeySetValues2.patch
Type: text/x-patch
Size: 30821 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150401/a79c1820/makePathsAndFilesObeySetValues2-0001.patch>


More information about the distro-pkg-dev mailing list