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