On 10/11/07, Ajith Ranabahu <[EMAIL PROTECTED]> wrote: > > Hi, > Sorry guys - I jumped into conclusions. I was mislead by the > documentation pointing to an older source (I was thinking that we did > a release but seems not - And the new code is pretty much different > from the old one). By going through the svn logs I made sure that the > changes has indeed been done in the trunk for the past few months. > Thanks Sanka for the clarifications > > While trying to implement some of the stuff I came across some points > in the code and so this would be the right time to point it out and > perhaps start a conversation around it. So here are some of the > thoughts for improvement > > 1. The AssertionBuilderFactory is made static inside the PolicyEngine. > It seems this is not flexible since I want to have my own defaults. It > would have been better if we can control this via a System property > but for now I made it a field and added a setter so that one can set a > custom factory
I think the authors should provide custom AssertionBuilder s rather than a custom AssertionBuilderFactory. And yes .. we should be able to specify how to locate such custom AssertionBuilders. For example we are using only Sun service provider mechanism at the moment. But I think we should also use a System.propertyand property.properties file to locate any custom AssertionBuilders in the system. 2. The AssertionBuilderFactory need not have any static methods . It > should be an instance inside the PolicyEngine (static right now) and > it has a static registerBuilder method which makes it really ugly. > Actually the static factory instance is never used to register the > builders. So I've made the registerBuilder method an instance > method and also modified the PolicyEngine to rightfully use the static > factory instance. Also moved the static intializer code to the > constructor. sounds good !! How about making the AssertionBuilderFactory a singleton. Then the PolicyEngine can get hold of an instance by AssertionBuilderFactory factory = AssertionBuilderFactory.getInstance(); and a user can add custom AssertionBuilders programmetically by AssertionBuilderFactory factory = AssertionBuilderFactory.getInstance(); factory.register(...); Also when AssertionBuilderFactory.getInstance() is called for the first time we can use Sun Service Provider interface, or a system property or a property file to locate the initail set of AssertionBuilders available in the system. 3. The element field in XMLPrimitiveAssertion has a default scope and > is impossible to extend in an outside package. So I've made that a > protected variable. sounds good ! I will create a Jira and attach the patch. If there would be no > downstream problems (WS-Security policy may be) we can go ahead with > the changes sure .. please do start a conversation in the mailing list so that we can hear what other has to say also .. Ajith > > > > On 10/10/07, Ajith Ranabahu <[EMAIL PROTECTED]> wrote: > > Hi all, > > While playing with Neethi for a one of my research related things I > > realized (somewhat painfully!) that the neethi trunk has not been > > updated at all. The code for 1_1 is included in the branch but those > > changes are not in the trunk !!! (The site does not point to the right > > SVN location BTW) > > > > From the log I see that there has been some activity with the branches > > around February but the latest has not been merged to the trunk. I'm > > thinking this is a mistake that 'fell through the cracks' and went > > unnoticed for a long time. Would it be possible to correct this ? As > > for the time I guess the code in the branch should be the right one to > > play with. > > > > -- > > Ajith Ranabahu > > > > Reading, after a certain age, diverts the mind too much from its > > creative pursuits. Any man who reads too much and uses his own brain > > too little falls into lazy habits of thinking - Albert Einstein > > > > > -- > Ajith Ranabahu > > Reading, after a certain age, diverts the mind too much from its > creative pursuits. Any man who reads too much and uses his own brain > too little falls into lazy habits of thinking - Albert Einstein > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > -- Sanka Samaranayake WSO2 Inc. http://www.bloglines.com/blog/sanka http://www.wso2.org/
