geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Jencks <david_jen...@yahoo.com>
Subject Re: GBeanName [was: svn commit: r154723...]
Date Wed, 23 Feb 2005 08:23:03 GMT
+1 on canonical name as internal string representation
-1 on attempting to preserve whatever string is used to construct the 
gbean name
+1 on restricting characters in gbean name and assuring that gbean name 
 >> object name conversion always works in a non-surprising manner.

(although I may change my mind, currently I am strongly in favor of the 
above)

thanks
david jencks

On Feb 23, 2005, at 12:13 AM, Dain Sundstrom wrote:

> On Feb 22, 2005, at 10:06 PM, Jeremy Boynes wrote:
>
>> Dain Sundstrom wrote:
>>> On Feb 22, 2005, at 5:38 PM, Jeremy Boynes wrote:
>>>> Dain Sundstrom wrote:
>>>>
>>>>> I think we will need a canonical form.
>>>>
>>>>
>>>> Why?
>>> Because, you want to print two gbeans names that are equal and get 
>>> the same string. Because, you don't want geronimo classes bleeding 
>>> into your apis.
>>> Anyway, why not? ObjectName has a canonical format, why are we not 
>>> providing one?
>>>
>>
>> Well, apart from flight of fancy we don't actually have a need for it 
>> yet. It's not exactly hard to add when we do.
>
> This is not an flight of fancy. The first argument is a desire to 
> preserve useful functionality that ObjectName provided (i.e., a string 
> from that is guaranteed to be equal only when the names are equal), 
> and the second argument is a desire to keep our objects from bleeding 
> into other people's apis (i.e., I don't want another qname fiasco).
>
>>>>> There are many places that key stuff off the string canonical form 
>>>>> of the object name.
>>>>
>>>>
>>>> Isn't that fundamentally a bad idea? If you want to key off GBean 
>>>> name, key off GBeanName and not the string representation.
>>> Is it faster to compare two strings or two strings and two hash 
>>> maps?  My guess is the former.
>>
>> That would be internal implementation detail in equals()
>
> Internal implementation matters unless you make GBeanName an interface 
> or non-final.  In the case of a single implementation class such as 
> this, the implementation we choose dictates performance.
>
>> The point is that if you are keying on GBeanName then use 
>> GBeanName.equals() rather then GBeanName.toString().equals()
>
> I understand you desire to have people to use the name as a key, but 
> we should not require it.  ObjectName has shown that providing a 
> simple string canonical form allows people to key without needing to 
> use their classes, and we should do the same.  Most importantly it 
> keeps user apis free of geronimo classes.
>
>>>>> I think that instead of keeping the original name around, we throw 
>>>>> it out and replace it with canonical name form, since the original 
>>>>> name doesn't really matter.
>>>>
>>>>
>>>> It does to the user - it means that for a String x
>>>> x.equals(new GBeanName(x).toString())
>>> Right, because they are not equal strings.  If you do x.equals(new 
>>> Float(x).toString()) it will no be true.
>>
>> I don't think approximate GBeanNames would be useful to anyone.
>
> My point is that your argument is invalid.  There is no guarantee in 
> java that x.equals(new SomeClass(x).toString()) is ever true.
>
>>> Anyway, if you want to keep the original form for this use case 
>>> (which I doubt we will ever see), the add a getCanonicalName method 
>>> like ObjectName has.
>>
>> ObjectName.getCanonicalName() sorts the keys lexically which does not 
>> keep the orginal form; toString()'s output format is undefined. 
>> That's the problem. ObjectName does not allow you to easily preserve 
>> the original name (you have to reconstruct it from getDomain() and 
>> getKeyPropertyListString()).
>>
>>>> Or specifically, that if you specify properties in some meaningful 
>>>> order the name does not get rearranged on you. So if you have a 
>>>> name in a plan that will be the name displayed in the console not 
>>>> some variant of it.
>>> The order should not be "meaningful".  If we are doing anything like 
>>> ObjectName, the order does not matter, nor do I think we should be 
>>> encouraging a meaning to order.
>>
>> Order is not meaningful to us, but it may be to the user so why 
>> should we rearrange it on them (especially when it doesn't matter to 
>> us)?
>
> Why keep it around if it is only for display purposes?  The reason I 
> ask, is an unordered name is useless for any other purpose.
>
> Well, having a canonical form is important, so this would mean we need 
> two strings.  If the group wants the GBeanName to carry two strings, 
> then that works.  I personally only want one string, canonical form.
>
>> For example, the current approach means that the example names from 
>> the JSR-77 spec would be displayed as they are specified; using the 
>> canonical form of ObjectName would rearrange them. The order of the 
>> parts is not meaningful to the implementation but it sure makes the 
>> examples easier to understand.
>
> 77 names are generated in the deployment code using unordered maps.  
> IMNHO, a console respecting 77 should sort the 77 names into a tree 
> based on the hierarchy laid out in the spec.
>
>>>> On the other hand, if you supply the properties using a Map, they 
>>>> are sorted into lexical order when constructing the String 
>>>> representation on the assumption that most Maps will be 
>>>> non-deterministically ordered (i.e. that in most cases the Map 
>>>> supplied will be a Properties object) and this provides a 
>>>> representation that is consistent between VMs/architectures.
>>> Playing devils advocate, what if I provide you a LinkedHashMap 
>>> containing a "meaningful" order.
>>
>> Because "in most cases the Map supplied will be a Properties object".
>>
>> We could of course check for LinkedHashMap but, also playing devil's 
>> advocate, the user could have their own implementation of Map.
>>
>> Or, we can add an ordered constructor e.g.
>> GBeanName(String domain, List<String> keys, List<String> values)
>> GBeanName(String domain, String[] keys, String[] values)
>>
>> Or, we can have two Map-type constructors e.g.
>> GBeanName(String domain, Properties props) // we reorder
>> GBeanName(String domain, Map props) // use order from iterator()
>>
>> The behaviour currently is clear and simple, and if they want a 
>> specific order they can always use the String constructor (because it 
>> preserves the value they supply). We don't need to overcomplicate 
>> this.
>
> I'm not going to press this point, other then to say I believe your 
> analysis supports my point.
>
>>>>> Also I would hope the canonical form is the same as object name.
>>>>
>>>>
>>>> That assumes a need for canonical name.
>>>>
>>>>> One other thing, it doesn't look like this is escaping key values, 
>>>>> or checking for illegal characters.
>>>>
>>>>
>>>> Nope - there is a significant performance overhead with 
>>>> javax.management.ObjectName in all the validation and esacaping it 
>>>> does of key/value pairs. We can avoid all of that by imposing the 
>>>> condition that ':', ',' and '=' are reserved characters and should 
>>>> not be used.
>>> That is fine with me because it is further restriction on the type. 
>>> But we must also include asterisk and question mark to not conflict 
>>> with object name patterns (and it would allow for really ugly 
>>> names).  Also we should disallow the quote character to not conflict 
>>> with quoted object names.
>>
>> Patterns work differently - ObjectName's overloading of real names 
>> and patterns is an abomination and we don't need to continue using 
>> it.
>
> That is for another discussion.  If we don't restrict asterisk and 
> question, we are expanding on the type which I am definitely -1 for as 
> it would make it impossible to mount some names into an mbean server.
>
>>> So are you going to add a validation phase to scan the string for 
>>> illegal characters?  We don't create names in critical sections of 
>>> code, so I wouldn't mind the over head.  Also it should be pretty 
>>> fast.
>>
>> I still don't see the need. The JMXGBeanRegistry will need to convert 
>> the names to valid ObjectName's to register/unregister instances with 
>> the MBeanServer but that is a very specific circumstance, and IMO a 
>> JMX problem. The BasicGBeanRegistry need not care.
>
> If we restrict characters, we should enforce the restriction.  
> Otherwise, names will be created that can not be mounted into JMX.  
> That means that someone could have a perfectly working system, turn on 
> jmx and nothing works.  I personally don't ever want to be in that 
> situation.
>
>>>> Any GBean can be registered with an MBeanServer simply by quoting 
>>>> the name, making escaping a JMX issue not a GBean one which should 
>>>> be handled in the JMX registry.
>>> If we take the restrictions above, there will not be need to escape 
>>> at all.
>>> -dain
>>
>> Can we be done with this bikeshed now?
>
> This is not a bikeshead; these are important decisions that should be 
> discussed.  Your decisions could break seamless integration with JMX, 
> and could require redesign of OpenEJB, ActiveMQ gbeans, and gbeans in 
> geronimo itself.
>
> If you want, I'll implement the class and make the changes.
>
> -dain
>


Mime
View raw message