Hi, I'm looking at RemoteCommandFactory in Infinispan as part as mapping the marshalling stuff to JBoss Marshalling and here's something that has confused me: fromStream() method says: * Creates an un-initialized command. Un-initialized in the sense that parameters will be set, but any components * specific to the cache in question will not be set. * <p/> * You would typically set these parameters using {@link org .infinispan .commands .CommandsFactory#initializeReplicableCommand(ReplicableCommand)} * <p/> And then you do the following: case StateTransferControlCommand.METHOD_ID: command = new StateTransferControlCommand(); ((StateTransferControlCommand) command).init(rpcManager); break; Shouldn't ((StateTransferControlCommand) command).init(rpcManager) go into org .infinispan .commands .CommandsFactory#initializeReplicableCommand(ReplicableCommand). initializeReplicableCommand() javadoc says: * Initializes a {@link org.infinispan.commands.ReplicableCommand} read from * a data stream with components specific to the target cache instance. The rpcManager seems to be one those components?

> > No. The RPCManager is scoped as GLOBAL: > https://svn.jboss.org/repos/infinispan/trunk/src/main/java/org/infinispan... > Refer to the @Scope annotation on the interface. > But I understand your confusion here. We should either: > 1. Change the Javadocs on fromStream() to be more explicit, i.e., > only dependent components of GLOBAL scope are injected. NAMED_CACHE > scope components are only injected later with > CommandsFactory.initializeReplicableCommand(). > 2. Remove the injection in fromStream(); have > CommandsFactory.initializeReplicableCommand() handle all of the > injection, of GLOBAL and NAMED_CACHE scoped components. > I have no problem with 2, if it makes your integration easier. In > fact, I actually think 2 is probably cleaner. Yeah, 2 would be perfect for me. I can implement 2 and run the full testsuite and see whether all works fine. If it looks good, I'll create a JIRA so that we keep a separate track of this change.

Manik Surtani wrote: > On 25 Mar 2009, at 19:23, Galder Zamarreno wrote: >> Hi, >> >> I'm looking at RemoteCommandFactory in Infinispan as part as mapping >> the marshalling stuff to JBoss Marshalling and here's something >> that has confused me: >> >> fromStream() method says: >> >> * Creates an un-initialized command. Un-initialized in the sense >> that parameters will be set, but any components >> * specific to the cache in question will not be set. >> * <p/> >> * You would typically set these parameters using {@link >> org >> .infinispan >> .commands >> .CommandsFactory#initializeReplicableCommand(ReplicableCommand)} >> * <p/> >> >> And then you do the following: >> >> case StateTransferControlCommand.METHOD_ID: >> command = new StateTransferControlCommand(); >> ((StateTransferControlCommand) command).init(rpcManager); >> break; >> >> Shouldn't ((StateTransferControlCommand) command).init(rpcManager) >> go into >> org >> .infinispan >> .commands >> .CommandsFactory#initializeReplicableCommand(ReplicableCommand). >> >> initializeReplicableCommand() javadoc says: >> >> * Initializes a {@link org.infinispan.commands.ReplicableCommand} >> read from * a data stream with components specific to the target >> cache instance. >> >> The rpcManager seems to be one those components? > No. The RPCManager is scoped as GLOBAL: > https://svn.jboss.org/repos/infinispan/trunk/src/main/java/org/infinispan... > Refer to the @Scope annotation on the interface. > But I understand your confusion here. We should either: > 1. Change the Javadocs on fromStream() to be more explicit, i.e., > only dependent components of GLOBAL scope are injected. > NAMED_CACHE scope components are only injected later with > CommandsFactory.initializeReplicableCommand(). > 2. Remove the injection in fromStream(); have > CommandsFactory.initializeReplicableCommand() handle all of the > injection, of GLOBAL and NAMED_CACHE scoped components. > I have no problem with 2, if it makes your integration easier. In > fact, I actually think 2 is probably cleaner. I've looked into this and it looked simpler saying than actually doing it. The problem here is that StateTransferControlCommand is dealt in a very different way to the rest of ReplicableCommands that extend CacheRPCCommand.

CommandAwareRpcDispatcher.handle shows http://pastebin.com/m446036c4 and the problem is that perform() is called directly for StateTransferControlCommand before any initialisation could be done. That's why the RPCManager was set during unmarshalling. I still think that StateTransferControlCommand, being a ReplicableCommand, should be allowed to go through CommandsFactory.initializeReplicableCommand() but CommandAwareRpcDispatcher.handle does not have access to command factory and neither the global registry of components.

Ideally, I'd like to see something this being doable: http://pastebin.com/m3f403e2 However, I dunno whether it's doable or make sense. Currently, CommandsFactory.initializeReplicableCommand() call happens within InboundInvocationHandlerImpl.handle(CacheRPCCommand). Thoughts? I suppose I could always fallback on 1 but I don't like different commands being treated in a different way. Ideally, all replicable commands received would be instantiated in the marshaller, and they would all be initialised in the same place. > Cheers > -- > Manik Surtani > Lead, JBoss Cache > http://www.jbosscache.org > manik(a)jboss.org -- Galder Zamarreño Sr. Software Maintenance Engineer JBoss, a division of Red Hat