You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@river.apache.org by Mark Brouwer <ma...@marbro.org> on 2008/05/19 21:41:56 UTC
Change to net.jini API
All,
Attached you will find a patch for RIVER-147 and RIVER-265. Note that
RIVER-147 represent a new protected method on a class in the net.jini
namespace. To me this a public API change in the net.jini namespace so
please all have a good look at it and tell what you think of it.
The enhancements represent the minimal specification of what is referred
to in RIVER-147, but the enhancements represent all that I needed to
support codebase evolution for Seven.
RIVER-265 represents an internal change that is a direct consequence of
the functionality provided by RIVER-147.
Regards,
--
Mark Brouwer
Re: Change to net.jini API
Posted by Peter Jones <pe...@sun.com>.
On Sat, May 31, 2008 at 05:27:07PM +0200, Mark Brouwer wrote:
> Peter Jones wrote:
>> One preliminary question: could you explain the thinking that led
>> you to making the codebase parameter to the new protected method be
>> a String instead of a URL[] (which will already have been computed
>> at the corresponding points in the implementation, as effectively
>> required by the spec)? I'm not asserting that this is the wrong
>> choice, just wondering what your motivation was, to help me think
>> about it. Thanks.
>
> I think the best thing to do is to read these thread from almost a
> year ago as a refresher
> http://mail-archives.apache.org/mod_mbox/incubator-river-dev/200706.mbox/%3c4673FEF7.3010703@cheiron.org%3e
> .
Thanks for the pointer/reminder.
> I hope this makes clear why I came up with the API as it is [1]. I
> think I fail to understand your remark "as effectively required by
> the spec", so could you give me an extra hint here as to what you
> are referring.
I was referring to the "throws MalformedURLException" specified for
PCP's loadClass, loadProxyClass, and getClassLoader methods, which I
interpret to mean that the codebase argument must be converted to URLs
in any case, early on.
> [1] from that thread I can understand people fail to see why the
> signature doesn't have both the codebase string and URL array but I
> decided in that year that I didn't like that.
My hesitation on what the signature of this new protected method
should be is partly related to the fact that PCP's implementation of
the proposed version makes use of facilities not exposed to
subclasses:
- the pathToURLs cache, both for the (String) codebase argument as
well as for the loader's annotation
- a straightforward way to get the annotation string for the loader.
The public getClassAnnotation(Class) method is not straightforward
because it requires a Class defined by the loader, and it checks
permissions for URLClassLoaders.
So if a subclass wanted to, e.g., customize the URL path matching
function but not the loader->annotation mapping, which I think is
closer to the original spirit of RIVER-147 (Bob?), that wouldn't be
straightforward to do with this hook. I gather that you do want to
customize the loader->annotation mapping, which is why the loader
parameter is essential for your purpose.
This makes me wonder, though-- you must be overriding
getClassAnnotation(Class) too, right? Would you be happier with a way
to override the getLoaderAnnotation method, which would cover both the
getClassAnnotation(Class) case and the boomerang matching case? And
with such a method exposed to subclasses in addition to the proposed
one, a different subclass could more easily override just the matching
function. (Of course, there is the fear that subclass contract design
in general is strewn with land mines, exponentially related to the
number of API elements involved in the contract...)
Following are the rest of my comments based on the currently proposed
API changes:
There will need to be some additional spec updates-- in particular, I
think that all references to a loader's "annotation URL path" will
need to be replaced:
- Conditions based on the equality of a URL path to a loader's
annotation URL path will need to be replaced with conditions based on
the result of invoking the new protected method.
- The remaining case is the definition of "permission to access the
codebase loader". With this change, it seems appropriate to replace
"codebase loader's annotation URL path" with "codebase URL path"-- no
longer the same thing in applicable cases-- because the latter are the
URLs whose permission will actually be checked.
With those replacements, it seems that the definition of a loader's
"annotation URL path" can be removed, although perhaps some of its
words should go into the specification of PCP's implementation of the
protected method.
The proposed spec for the new method mentions the term "class
boomerang", but that's not a term defined or used elsewhere in the
spec, and I'm inclined to keep it that way. :-)
Since the 2.1 version, I think that all of PCP's spec words are in my
"voice", so if you like, for consistency there, I would be happy to
supply final words for these spec updates, once the signature and
semantics are settled.
Regarding the proposed name of the new protected method:
+ protected boolean isClassDefinedInClassLoader(String codebase,
+ ClassLoader loader) {
A long time ago, a certain JVM spec author drilled into my head that a
class is not defined "in" a class loader, it is defined "by" a class
loader, so I don't like the use of "in" here. Moreover, I agree with
a previous comment that it seems odd for the name to mention a class
when the parameters do not identify a particular class. I suggest the
name "isCodebaseForClassLoader". Thoughts?
Regarding the proposed changes to the checkLoader method: it seems odd
to me for the (optionally-enabled) assertion to invoke this
subclass-overridable method-- especially since the "functional"
requirements of the method are not entirely clear. I would just
remove this assertion from checkLoader, and then its parameter list
can remain as it was.
@@ -481,7 +481,6 @@
}
URL[] codebaseURLs = pathToURLs(codebase); // may be null
- // throws MalformedURLException
It's a trivial matter, but I have to ask: why these comment removals
(in three places)? They were there to clarify for the reader/author
where the MalformedURLException requirement for these methods is
implemented.
Remaining code changes look OK to me.
-- Peter
Re: Change to net.jini API
Posted by Mark Brouwer <ma...@marbro.org>.
Hi Peter,
Peter Jones wrote:
>
> On May 31, 2008, at 6:32 AM, Mark Brouwer wrote:
>> Mark Brouwer wrote:
>>> All,
>>> Attached you will find a patch for RIVER-147 and RIVER-265. Note that
>>> RIVER-147 represent a new protected method on a class in the net.jini
>>> namespace. To me this a public API change in the net.jini namespace so
>>> please all have a good look at it and tell what you think of it.
>>> The enhancements represent the minimal specification of what is referred
>>> to in RIVER-147, but the enhancements represent all that I needed to
>>> support codebase evolution for Seven.
>>> RIVER-265 represents an internal change that is a direct consequence of
>>> the functionality provided by RIVER-147.
>>
>> Did anyone have had a look at this change, I don't want to commit this
>> given the fact it is a change in the net.jini API which is rather hard
>> to remove if it turns out it ain't that smart after all.
>
> Yes, I finally got a chance to start looking at this yesterday, and I
> hope to write up a few comments in the next day or so (I don't have much
> time at the this immediate moment, though).
>
> One preliminary question: could you explain the thinking that led you to
> making the codebase parameter to the new protected method be a String
> instead of a URL[] (which will already have been computed at the
> corresponding points in the implementation, as effectively required by
> the spec)? I'm not asserting that this is the wrong choice, just
> wondering what your motivation was, to help me think about it. Thanks.
I think the best thing to do is to read these thread from almost a year
ago as a refresher
http://mail-archives.apache.org/mod_mbox/incubator-river-dev/200706.mbox/%3c4673FEF7.3010703@cheiron.org%3e
. I hope this makes clear why I came up with the API as it is [1]. I
think I fail to understand your remark "as effectively required by the
spec", so could you give me an extra hint here as to what you are referring.
[1] from that thread I can understand people fail to see why the
signature doesn't have both the codebase string and URL array but I
decided in that year that I didn't like that.
--
Mark
Re: Change to net.jini API
Posted by Peter Jones <Pe...@Sun.COM>.
On May 31, 2008, at 6:32 AM, Mark Brouwer wrote:
> Mark Brouwer wrote:
>> All,
>> Attached you will find a patch for RIVER-147 and RIVER-265. Note that
>> RIVER-147 represent a new protected method on a class in the net.jini
>> namespace. To me this a public API change in the net.jini
>> namespace so
>> please all have a good look at it and tell what you think of it.
>> The enhancements represent the minimal specification of what is
>> referred
>> to in RIVER-147, but the enhancements represent all that I needed to
>> support codebase evolution for Seven.
>> RIVER-265 represents an internal change that is a direct
>> consequence of
>> the functionality provided by RIVER-147.
>
> Did anyone have had a look at this change, I don't want to commit
> this given the fact it is a change in the net.jini API which is
> rather hard to remove if it turns out it ain't that smart after all.
Yes, I finally got a chance to start looking at this yesterday, and I
hope to write up a few comments in the next day or so (I don't have
much time at the this immediate moment, though).
One preliminary question: could you explain the thinking that led you
to making the codebase parameter to the new protected method be a
String instead of a URL[] (which will already have been computed at
the corresponding points in the implementation, as effectively
required by the spec)? I'm not asserting that this is the wrong
choice, just wondering what your motivation was, to help me think
about it. Thanks.
-- Peter
Re: Change to net.jini API
Posted by Mark Brouwer <ma...@marbro.org>.
Hi Wade,
Wade Chandler wrote:
> --- On Sat, 5/31/08, Mark Brouwer <ma...@marbro.org> wrote:
>> From: Mark Brouwer <ma...@marbro.org>
>> Subject: Re: Change to net.jini API
>> To: river-dev@incubator.apache.org
>> Date: Saturday, May 31, 2008, 6:32 AM
>> Mark Brouwer wrote:
>>> All,
>>>
>>> Attached you will find a patch for RIVER-147 and
>> RIVER-265. Note that
>>> RIVER-147 represent a new protected method on a class
>> in the net.jini
>>> namespace. To me this a public API change in the
>> net.jini namespace so
>>> please all have a good look at it and tell what you
>> think of it.
>>> The enhancements represent the minimal specification
>> of what is referred
>>> to in RIVER-147, but the enhancements represent all
>> that I needed to
>>> support codebase evolution for Seven.
>>>
>>> RIVER-265 represents an internal change that is a
>> direct consequence of
>>> the functionality provided by RIVER-147.
>> Did anyone have had a look at this change, I don't want
>> to commit this
>> given the fact it is a change in the net.jini API which is
>> rather hard
>> to remove if it turns out it ain't that smart after
>> all.
>
> I meant to point out that I didn't get:
> protected boolean isClassDefinedInClassLoader(String codebase,
> ClassLoader loader)
>
If there is a better method name I'm all for it, I think the method
level javadoc gives a better description of what it does and the name
was the best at a certain evening I could come up with.
> as it seems per the method name the parameter codebase would be
> className. Is that what it was supposed to be, or is the intended use?
What is passed in is really the same argument (codebase) as what is
passed in at the call below:
PreferredClassProvider.loadClass(String codebase,
String name,
ClassLoader defaultLoader)
So it is really the codebase as found in a marshalled stream e.g.
--
Mark
Re: Change to net.jini API
Posted by Wade Chandler <hw...@yahoo.com>.
--- On Sat, 5/31/08, Mark Brouwer <ma...@marbro.org> wrote:
> From: Mark Brouwer <ma...@marbro.org>
> Subject: Re: Change to net.jini API
> To: river-dev@incubator.apache.org
> Date: Saturday, May 31, 2008, 6:32 AM
> Mark Brouwer wrote:
> > All,
> >
> > Attached you will find a patch for RIVER-147 and
> RIVER-265. Note that
> > RIVER-147 represent a new protected method on a class
> in the net.jini
> > namespace. To me this a public API change in the
> net.jini namespace so
> > please all have a good look at it and tell what you
> think of it.
> >
> > The enhancements represent the minimal specification
> of what is referred
> > to in RIVER-147, but the enhancements represent all
> that I needed to
> > support codebase evolution for Seven.
> >
> > RIVER-265 represents an internal change that is a
> direct consequence of
> > the functionality provided by RIVER-147.
>
> Did anyone have had a look at this change, I don't want
> to commit this
> given the fact it is a change in the net.jini API which is
> rather hard
> to remove if it turns out it ain't that smart after
> all.
I meant to point out that I didn't get:
protected boolean isClassDefinedInClassLoader(String codebase,
ClassLoader loader)
as it seems per the method name the parameter codebase would be className. Is that what it was supposed to be, or is the intended use?
Wade
==================
Wade Chandler, CCE
Software Engineer and Developer, Certified Forensic Computer Examiner, NetBeans Dream Team Member, and NetBeans Board Member
http://www.certified-computer-examiner.com
http://wiki.netbeans.org/wiki/view/NetBeansDreamTeam
http://www.netbeans.org
Re: Change to net.jini API
Posted by Mark Brouwer <ma...@marbro.org>.
Mark Brouwer wrote:
> All,
>
> Attached you will find a patch for RIVER-147 and RIVER-265. Note that
> RIVER-147 represent a new protected method on a class in the net.jini
> namespace. To me this a public API change in the net.jini namespace so
> please all have a good look at it and tell what you think of it.
>
> The enhancements represent the minimal specification of what is referred
> to in RIVER-147, but the enhancements represent all that I needed to
> support codebase evolution for Seven.
>
> RIVER-265 represents an internal change that is a direct consequence of
> the functionality provided by RIVER-147.
Did anyone have had a look at this change, I don't want to commit this
given the fact it is a change in the net.jini API which is rather hard
to remove if it turns out it ain't that smart after all.
--
Mark
Re: Change to net.jini API
Posted by Mark Brouwer <ma...@marbro.org>.
Mark Brouwer wrote:
> All,
>
> Attached you will find a patch for RIVER-147 and RIVER-265. Note that
> RIVER-147 represent a new protected method on a class in the net.jini
> namespace. To me this a public API change in the net.jini namespace so
> please all have a good look at it and tell what you think of it.
>
> The enhancements represent the minimal specification of what is referred
> to in RIVER-147, but the enhancements represent all that I needed to
> support codebase evolution for Seven.
>
> RIVER-265 represents an internal change that is a direct consequence of
> the functionality provided by RIVER-147.
>
> Regards,
>
Forgot to mention that I integrated your spelling corrections Peter, so
no need to worry about that ...
--
Mark