Yep I agree about seperating the tests into a seperate folder. I'll take that as a TODO Thanks!
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ "Those are my principles. If you don't like them I have others." http://incubator.apache.org/imperius Neeraj Joshi Autonomic Computing Policy Development Tivoli, IBM ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Craig L Russell <[EMAIL PROTECTED]> Sent by: [EMAIL PROTECTED] 01/03/2008 11:44 AM Please respond to [email protected] To [email protected] cc Subject Re: misc questions part 2 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 <[EMAIL PROTECTED]> > > Date: Thu, 3 Jan 2008 09:48:09 > To:[email protected] > 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:[EMAIL PROTECTED] P.S. A good JDO? O, Gasp!
