incubator-imperius-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Craig L Russell <Craig.Russ...@Sun.COM>
Subject Re: misc questions part 2
Date Thu, 03 Jan 2008 16:44:48 GMT

On Jan 3, 2008, at 7:32 AM, Erik Bengtson wrote:

> 14) what's the coding style in use? Do you have a formatter for  
> eclipse?
>
> I want to provide patches with the correct formatting
>
> 10) someplaces don't have. I also suggest removing  
> thread.currentThread.getname from logs because you can setup a  
> logging pattern that adds that transparently.
>
> 12) I think Craig can add more input to that than me, but  
> separating tests from main code might be a good idea.
>
Yes, the structure under src should allow the project to define test  
cases and other resources for test and for distribution.

Take a look at http://docs.codehaus.org/display/MAVEN/best+practices+- 
+testing+strategies

In particular, the directory structure
trunk/
   |_ src
     |_ main
       |_ java
       |_ resources

     |_ test
       |_ java
       |_ resources

If more complex integration testing is needed, then each such  
integration test would have its own sub-project. But imperius might  
not need this level of complexity.

Craig
>
> --   BlackBerry® from Mobistar    ---
>
> -----Original Message-----
> From: Neeraj Joshi <jneeraj@us.ibm.com>
>
> Date: Thu, 3 Jan 2008 09:48:09
> To:imperius-dev@incubator.apache.org
> Subject: Re: misc questions part 2
>
>
> Hi Erik,
> 3) You are right. For some reason I thought you meant a GUI.
>
> 4 & 6) I think its a great idea it will simplify the user's code.  
> Actually
> let me take a crack
> at it. So the 2 changes would be as follows: 1) The user provides a  
> map
> with instancename -> instanceobject
> and 2) the evaluator will silently ignore instances that are not  
> imported
> inside the policy
>
> 7)  I agree XML for the customexpressions is more elegant than  
> properties,
> I also agree about externalizing all the in-built expressions
> (acplparsermap) as an XML. Looking
> forward to your patch.
>
> 8) Good point I will add the privileged blocks whereever required
>
> 9 & 13) Will look into the logging and cleaning up code
>
> 10) I think most of the places we do have statements like this:
>
> if(logger.isLoggable(Level.FINE))
> logger.fine(Thread.currentThread().getName()+" iterating over  
> instances "
> );
>
> Is that not sufficient or have we missed putting the if in some  
> places?
>
> 11) As a matter of fact in our original ANT build we had it built as a
> seperate jar but then while migrating to maven
> in the interest of time we put it all together. Will work on  
> seperating it
> out
>
> 12) I suppose this is the maven convention? Again in the interest  
> of time
> I didn't change the folder structure, do you see a
> compelling reason for this? I noticed some other projects like  
> Apache Muse
> weren't following it either
>
> 14) I personally don't prefer the Apache Geronimo style but if  
> everyone
> feels strongly about this I am OK with changing it.
>
> 15) Please do provide a patch.
>
> Again we appreciate your in depth feedback!
> Neeraj
>
>
>
>
>
>
>
>
>
> 3)
>> NRJ: The factory is a good idea. What do you have in mind in terms  
>> of a
>> user interface?
>
> I just thought in rationalizing the current APIs and make all user  
> exposed
> methods interface based.
>
> If I understand correctly, there are two kinds of API: User API,  
> Service
> Provider Implementation API
>
> User API is the user view to imperius
> SPI API is for instance the datastore interfaces, mostly the one in  
> the
> external
> packages.
>
> 4)
>> NRJ- There is a direct correlation between the arguments passed to  
>> the
>> evaluatePolicy method and the imported classes/instances within the
>> policy.
>> For e.g.
>>
>> If the import statement in the policy is as follows:
>>
>> Import Class a : a1,a2;
>> Import Class b: b1,b2;
>>
>> Then the evaluatePolicy method call  would have inputMap as the  
>> parameter
>>
>> where
>> Map inputMap =  ["a" --> instanceInfoListForA]
>>                ["b" --> instanceInfoListForB ]
>>
>> where
>> instanceInfoListForA = [instanceInfoA1  , instanceInfoA2]
>> instanceInfoLIstForB = [instanceInfoB1 , instanceInfoB2 ]
>
> IMO non declared instanceinfo objects could be silently ignored. e.g.
>
> <source>
> Map inputMap =  ["a" --> instanceInfoListForA]
>                 ["b" --> instanceInfoListForB ]
>                 ["c" --> instanceInfoListForC ]
>
>
> where
> instanceInfoListForA = [instanceInfoA1  , instanceInfoA2]
> instanceInfoLIstForB = [instanceInfoB1 , instanceInfoB2,  
> instanceInfoB3 ]
> instanceInfoLIstForC = [instanceInfoC1 ]
> </source>
>
> B3 and C1 could be silently ignored since the policy has:
>
>> Import Class a : a1,a2;
>> Import Class b: b1,b2;
>
> I can create a patch if you agree.
>
> 6) Simplified User API by hiding InstanceInfo?
>
> e.g.
>
> Map map = new HashMap();
> map.put("a1", myobjecta1);
> map.put("a2", myobjecta2);
> map.put("b1", myobjectb1);
> map.put("b2", myobjectb2);
>
> spl.executePolicy(name,map);
>
> Internally, PolicyManager.executePolicy can create InstanceInfo.
>
>
> 7) Properties Loader (customexpressions): Currently custom  
> expressions are
> configured in a property file placed at the user.dir or splhome. I  
> propose
> the
> following:
>
> - change properties to xml format
> - default naming of the file would change from
> customexpressions.properties to
> imperius.xml
> - imperius.xml would be loaded by default from the locations: cur.dir,
> user.dir
> and splhome.
> - allow configuring customexpressions via the SPLPolicyRuleProvider
> interface.
> The operation void registerExpressions(InputStream xml) is added to  
> the
> SPLPolicyRuleProvider.
>
> <imperius>
> <configuration>
> <spl-expression class-name="sample.SendMail"/>
> <spl-expression class-name="sample.ExecuteCommand"/>
> </configuration>
> <imperius>
>
> - Externalize ACPLParserMap expressions registration to another XML  
> file:
> org.apache.imperius.spl.Expressions.xml
>
> I can also provide a patch.
>
> 8) Access to protected resources. System.getProperty,  
> Class.forName, etc
>
> None of the access to these resources happen in privilege blocks, so
> imperius
> won't work in secured environment.
>
> 9) Cosmetic: There are plenty of messages output to system.out. These
> should be
> moved to log.
>
> 10) Logging code should be enclosed within a check condition:
>
> e.g.
>
> if( logger.isDebugEnabled() )
> {
>  logger.debug(xxxx)
> }
>
> 11) Samples in the JavaSPL should be moved to its own module. I  
> propose
> javaspl-samples
>
> 12) Unit tests should be moved to /src/test/
>
> 12) Sources should be moved from /src/ to /src/java/ or /src/main/
>
> 13) Cleanup code: Remove commented out code; Remove empty methods  
> such as
> static
> mains..; make classes private when possible
>
> 14) Code Formatting: use of Apache Geronimo coding style?
>
> 15) Prevent nullpointerexceptions. in some cases I got NPE, so need to
> prevent
> these situations. I propose to create unit tests for the test cases  
> I have
> and
> provide a patch.
>
> Regards
>
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> "Those are my principles. If you don't like them I have others."
>
> Neeraj Joshi
> Autonomic Computing Policy Development
> Tivoli, IBM
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Mime
View raw message