<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">I've implemented a POC as per your suggestions, my comments bellow.<br><div><div>On 29 Sep 2010, at 20:06, Manik Surtani wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>So this is an extension to the discussion around a GenericCommand that has been going around. &nbsp;IMO a GenericCommand is a big -1 from me for various reasons - the whole purpose of the command pattern is so we have strongly typed and unit testable commands. &nbsp;This will help the ongoing work by Mircea, Sanne and Israel on various modules that need to define custom commands.<br><br>I proposed the following solution to Mircea earlier today, I'll repeat here for you guys to discuss. &nbsp;Note that this is a *half baked* solution and needs more thought! &nbsp;:-)<br><br>* If a module needs to define custom commands, it should define its own ReplicableCommand implementations in its' own module.<br>* It should define a sub-interface to Visitor (MyModuleVisitor) with additional methods to handle the new commands<br>* Interceptors defined in this module should extend CommandInterceptor AND implement MyModuleVisitor</div></blockquote><blockquote type="cite"><div>* These new commands can be created directly, or via a new CommandFactory specially for these commands.<br></div></blockquote>directly should not be a problem.&nbsp;<br><blockquote type="cite"><div><br>Now for the un-finished bits. &nbsp;:)<br><br>* How does RemoteCommandFactory instantiate these new commands? &nbsp;The module should have a way of registering additional command IDs with RemoteCommandFactory.fromStream(). &nbsp;See <br><br><a href="http://fisheye.jboss.org/browse/Infinispan/branches/4.2.x/core/src/main/java/org/infinispan/commands/RemoteCommandsFactory.java?r=2327#l59">http://fisheye.jboss.org/browse/Infinispan/branches/4.2.x/core/src/main/java/org/infinispan/commands/RemoteCommandsFactory.java?r=2327#l59</a><br></div></blockquote></div><div><blockquote type="cite"><div><br>Perhaps RemoteCommandFactory.fromStream() should look up the ID in a map of command creator instances, and each module can register more of these with the RemoteCommandFactory?<br></div></blockquote><div>that's a more complex issue and has to do with managing the serialization of custom commands through our marshalling framework. Right now we cannot register marshaller at runtime, until&nbsp;<a href="https://jira.jboss.org/browse/ISPN-244">https://jira.jboss.org/browse/ISPN-244</a>&nbsp;is implemented.</div><div>My POC is based on java serialization - far from being optimal it can represent an approach till 5.0 is released.<br><blockquote type="cite"></blockquote></div><blockquote type="cite"><div><br>* How do interceptors defined in the core module handle commands it isn't aware of? &nbsp;handleDefault()? &nbsp;</div></blockquote>what about just invokeNext? this would make only the interceptors that are extensions to do any work.<br><blockquote type="cite"><div>Or should we define a new handleUnknown() method in Visitor for this case, which would default to a no-op in AbstractVisitor? &nbsp;E.g., in a module-specific command such as MyModuleCommand, I would implement:<br></div></blockquote>works as well.&nbsp;<br><blockquote type="cite"><div><br>class MyModuleCommand implements ReplicableCommand {<br><br> public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {<br> &nbsp;&nbsp;&nbsp;&nbsp;if (Visitor instanceof MyModuleVisitor) {<br> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return ((MyModuleVisitor) visitor).visitMyModuleCommand(ctx, this);<br> &nbsp;&nbsp;&nbsp;&nbsp;} else {<br> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;return visitor.handleUnknown(ctx, this);<br> &nbsp;&nbsp;&nbsp;&nbsp;}<br> &nbsp;}<br><br>}<br><br>Cheers<br>Manik<br><br>PS: There is no JIRA for this. &nbsp;If we like this approach and it works, I suggest we create a JIRA and implement it for 4.2. &nbsp;The impl should be simple once we resolve the outstanding bits.<br></div></blockquote>there is one actually:&nbsp;<a href="https://jira.jboss.org/browse/ISPN-256">https://jira.jboss.org/browse/ISPN-256</a></div><div>I'll attache the patches to it.</div><div><a href="https://jira.jboss.org/browse/ISPN-256"></a>Just to conclude, I've POC both approaches GenericCommand and ExtendingCommand, here are my thoughts on it:</div><div>- extending visitor</div><div>PROS: &nbsp;ExtendingCommand is a better OO API, more strongly typed</div><div>CONS: it is harder for the user to understand; integrating custom serialization might be difficult(impossible at the time), also it will have to manage command uniques.&nbsp;</div><div><br></div><div>- GenericCommand</div><div>CONS:&nbsp;</div><div>- less OO API</div><div>- would require a "convention" for not "overusing" it. Overuse would be in client code though, not in ours</div><div>PROS:</div><div>- easier for the user to understand and use</div><div><span class="Apple-tab-span" style="white-space:pre">        </span>-&nbsp;no need for extra classes needed</div><div><span class="Apple-tab-span" style="white-space:pre">        </span>- no&nbsp;understanding of how the interceptor chain internals work needed (for writing acceptVisitor).&nbsp;</div><div><span class="Apple-tab-span" style="white-space:pre">        </span>- no understanding of&nbsp;how serialization works &nbsp;is needed. Nor command uniques.</div><div>- easy streight-forward solutions</div><div><br></div><div>I would go for GenericCommand, but Extending approach works for me as well.</div><div><br></div><div>Cheers,</div><div>Mircea<br><blockquote type="cite"><div>--<br>Manik Surtani<br><a href="mailto:manik@jboss.org">manik@jboss.org</a><br>Lead, Infinispan<br>Lead, JBoss Cache<br>http://www.infinispan.org<br>http://www.jbosscache.org<br><br><br><br><br><br>_______________________________________________<br>infinispan-dev mailing list<br>infinispan-dev@lists.jboss.org<br>https://lists.jboss.org/mailman/listinfo/infinispan-dev<br></div></blockquote></div><br></body></html>