I mostly like your idea and implementation . What I like is using same common API to enable WS-Security , MTOM , WS-Addressing and WS-RM. What I do not make sure I like is it mixed the the wise core runtime configuration with user configuration for enable WS-*.

When user uses or integrate Wise , he or she will import the jars related to wise runtime .
The jboss-beans.xml will be packaged into jar file. We should not expect user unzip that jar and modify the META-INF/jboss-beans.xml to enable WS-security. If it's feasible, we should provide a separated configuration file for user to add WS-* configuration .

I see you introduced new class WSExtensionVisitor , but I do not find out where it will visit the WSEndpoint class. Now the behavior is WSSecurityEnabler/MTOMEnabler invoking the related method in WSExtensionVisitor and the method in WSExtensionVistor is responsible for do the real job to enable WS-*. So I say WSExtensionVistor does not really visit the WSEndpoint . Do you plan to modify this API to do something like this :

 public class WSExtensionVistor {
 public void visitWSSecurity() {
 ....
 }

 public void visitWSRM() {
 ....
 }

 public void visitWSAddressing() {
 ....
 }

 public void visit(WSEndpoint point ) {
 visitWSSecurity();
 visitWSRM();
 visitWSAddressing();
 }

 }

"maeste" wrote:

I don't agree here. jboss-beans.xml is the only config file where isr put their own configurations like "keepsource", "verbose" WS-* related and so on. They are specific "user space" configuration that final user have to set up for his own needs. I think wise-core.jar must not include jboss-beans.xml into its jar, but we have to ask users to put their own jboss-beans.xml as part of their project (you are right here we have to explain that better in our user manual).
IOW jboss-beans.xml is substituting wise-core.properties, the former way to define user's configurations.

"maeste" wrote:

Ok you are right it isn't a perfect visitor pattern. In fact I started to write this code thinking to just add WSExtensions to endpoint and implement a "visitAll" method which would scan the WSExtension collection to call enable and so visit all the element (the extension). But it was one more call in charge of users, so I decide to "visit" single elements during addition...I know it's a border line implementation of visitor pattern (or perhaps it isn't a visitor at all), but I think it could work for us and keep it simpler. Maybe we can rename this class? Or maybe not. Opinions?

"maeste" wrote:

Well, but we really need to make NativeSecurityConfig easily extensible (if jbossws sec will not change, I think it will not ever change too), loosing part of its easy readability? I made a value object here because I think it's better to have code easy to read and understand and easy to IOC (and it is in general my style, but I'd like to discuss about). With a prperty map users have to know valid value for keys while a value Object auto explain itself. As said it's just a matter of style, no problem for me to change it if you can see real advantages somewhere.

"maeste" wrote:

Oki, I'm fine with. I haven't in mind any pattern closer to this implementation, so a name like EnablerImplDelegate is fine for you? Something better in mind?

"maeste" wrote:

Yes you can inject Map. With "various style WS-Security" do you mena different implementation (native, cxf, metro)?
I would prefer to have different config class for different style of WS-Security, it keep it simple to understand also injection. With a map used for all style it's harder for users to understand which key/value pair have to inject. With multiple classes named NativeWSSecurityConfig, CXFSecurityConfig, MetroSecurityConfig it's easier for users to understand what they are injecting and javadoc full document the valid field out of the box.

"maeste" wrote:

But it stay in xml config file (the default for native). This IOC class purpose is only to give Wise client URL to config file, keystore, truststore, and config name to use. Specific config continue to stay in xml config, I don't think wise should wrap (at least at this stage) stack specific config files.

My 2 cents on the ws-security configuration being discussed here; if the Wise client wsse configuration (currently NativeSecurityConfig) is going to contain all the configuration items currently found in jboss-ws-security.xsd (http://fisheye.jboss.org/browse/JBossWS/stack/native/trunk/modules/core/src/main/resources/schema/jboss-ws-security_1_0.xsd?r=7167), well I don't think a map is a good solution, at least it's not the most elegant one. There're a lot of complexType even having children with the same names. As a matter of fact that xsd is currently mapped to a hierarchy of classes in JBossWS.

"alessio.soldano@jboss.com" wrote:
My 2 cents on the ws-security configuration being discussed here; if the Wise client wsse configuration (currently NativeSecurityConfig) is going to contain all the configuration items currently found in jboss-ws-security.xsd (http://fisheye.jboss.org/browse/JBossWS/stack/native/trunk/modules/core/src/main/resources/schema/jboss-ws-security_1_0.xsd?r=7167), well I don't think a map is a good solution, at least it's not the most elegant one. There're a lot of complexType even having children with the same names. As a matter of fact that xsd is currently mapped to a hierarchy of classes in JBossWS.