From 36a98f8794b49390b0f9215a449c124f043f067f Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Fri, 31 Dec 2010 21:56:46 +0000 Subject: [PATCH] Removed the need to supply a Client reference when making invocation service calls on the Java side, to match this added convenience we introduced to the ActionScript side some time ago. Before arms are raised into the air and waved like they really do care, bear the following in mind: Nothing will change at all on your project until you take some action to do so. That action could include any of the following: 1. Continue to use narya-tools-1.2 to run genservice and you'll get the old style "pass a Client reference for every service method invocation" marshallers and you can continue to live happily in the past. 2. Leave some of your stuff un-regenerated, but generate new stuff using the new tools and use the Client-reference-free style for new services. This is likely to engender some PITA if you're trying to use both styles in the same project because genservice just wants to operate on everything, but I offer it as an option in case some other PITA motivates the tolerance of this (presumably smaller) PITA. 3. Embrace the brave new world of simplicity and wield your good friend sed for fun and profit: (make necessary build.xml updates to start using narya-tools-1.4-SNAPSHOT) % find src -name '*Service.java' | xargs sed -i 's:Client client, ::g' (this will remove the Client argument from most of your services, some manual fixes may be needed) % ant genservice % find src -name '*.java' | xargs sed -i 's:_ctx.getClient(), ::g' (this will fix most of the places that pass a client reference into an invocation service, some manual fixes will probably be needed) I did this on Vilya and it magically took care of like 95% of the places where changes were needed. The remaining half dozen changes were painless. Clearly something like Yohoho vastly dwarfs Vilya in scope, but someone looking for an hour or two of mindless typing to distract them on a lazy Sunday afternoon would likely breeze through even that vast codebase without breaking a sweat. Two more notes: 1. I'm going to switch Narya and Vilya over to the new style, and I'll be fixing all the projects that use their services. 2. I lied (a teeny bit) about nothing changing. I made the old Client-taking sendRequest methods deprecated, so projects that don't switch to the new style will be presented with a large number of deprecation warnings. If this is too terrible a burden to bear, I can remove the deprecation annotations on those methods. git-svn-id: svn+ssh://src.earth.threerings.net/narya/trunk@6395 542714f4-19e9-0310-aa3c-eee0fc999fb1 --- .../presents/client/BlockingCommunicator.java | 2 +- .../client/ClientObjectInputStream.java | 44 ++++++++++ .../presents/data/InvocationMarshaller.java | 35 +++++++- .../server/net/PresentsConnection.java | 13 ++- .../server/net/ServerCommunicator.java | 7 ++ .../presents/tools/InvocationTask.java | 81 ++++++------------- .../threerings/presents/tools/dispatcher.tmpl | 2 +- .../threerings/presents/tools/marshaller.tmpl | 8 +- .../presents/tools/marshaller_as.tmpl | 8 +- .../threerings/presents/tools/provider.tmpl | 2 +- .../threerings/presents/tools/service_as.tmpl | 2 +- 11 files changed, 131 insertions(+), 73 deletions(-) create mode 100644 src/main/java/com/threerings/presents/client/ClientObjectInputStream.java diff --git a/src/main/java/com/threerings/presents/client/BlockingCommunicator.java b/src/main/java/com/threerings/presents/client/BlockingCommunicator.java index 045c6a449..2f8d660ce 100644 --- a/src/main/java/com/threerings/presents/client/BlockingCommunicator.java +++ b/src/main/java/com/threerings/presents/client/BlockingCommunicator.java @@ -652,7 +652,7 @@ public class BlockingCommunicator extends Communicator _fout = new FramingOutputStream(); // create our object input and output streams - _oin = new ObjectInputStream(_fin); + _oin = new ClientObjectInputStream(_client, _fin); _oin.setClassLoader(_loader); _oout = new ObjectOutputStream(_fout); } diff --git a/src/main/java/com/threerings/presents/client/ClientObjectInputStream.java b/src/main/java/com/threerings/presents/client/ClientObjectInputStream.java new file mode 100644 index 000000000..3774cb389 --- /dev/null +++ b/src/main/java/com/threerings/presents/client/ClientObjectInputStream.java @@ -0,0 +1,44 @@ +// +// $Id$ +// +// Narya library - tools for developing networked games +// Copyright (C) 2002-2010 Three Rings Design, Inc., All Rights Reserved +// http://code.google.com/p/narya/ +// +// This library is free software; you can redistribute it and/or modify it +// under the terms of the GNU Lesser General Public License as published +// by the Free Software Foundation; either version 2.1 of the License, or +// (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +package com.threerings.presents.client; + +import java.io.InputStream; + +import com.threerings.io.ObjectInputStream; + +/** + * A specialized {@link ObjectInputStream} used in conjunction with {@link Client} to allow + * instances that are read from the stream to obtain a client reference "on their way in". We use + * this to allow invocation marshallers to get a reference to the client with which they are + * associated when they are streamed in over the network. + */ +public class ClientObjectInputStream extends ObjectInputStream +{ + /** The client with which this input stream is associated. */ + public final Client client; + + public ClientObjectInputStream (Client client, InputStream source) + { + super(source); + this.client = client; + } +} diff --git a/src/main/java/com/threerings/presents/data/InvocationMarshaller.java b/src/main/java/com/threerings/presents/data/InvocationMarshaller.java index d959d1b9a..0df3eeb11 100644 --- a/src/main/java/com/threerings/presents/data/InvocationMarshaller.java +++ b/src/main/java/com/threerings/presents/data/InvocationMarshaller.java @@ -21,9 +21,14 @@ package com.threerings.presents.data; +import java.io.IOException; + +import com.threerings.io.ObjectInputStream; import com.threerings.io.Streamable; import com.threerings.presents.client.Client; +import com.threerings.presents.client.ClientObjectInputStream; +import com.threerings.presents.client.InvocationDirector; import com.threerings.presents.client.InvocationService; import com.threerings.presents.dobj.DObjectManager; import com.threerings.presents.dobj.InvocationResponseEvent; @@ -243,6 +248,16 @@ public class InvocationMarshaller } } + /** + * A custom reader method for {@link Streamable}. + */ + public void readObject (ObjectInputStream in) + throws IOException, ClassNotFoundException + { + in.defaultReadObject(); + _invdir = ((ClientObjectInputStream)in).client.getInvocationDirector(); + } + @Override public String toString () { @@ -253,15 +268,27 @@ public class InvocationMarshaller * Called by generated invocation marshaller code; packages up and sends the specified * invocation service request. */ - protected void sendRequest (Client client, int methodId, Object[] args) + protected void sendRequest (int methodId, Object[] args) { - sendRequest(client, methodId, args, Transport.DEFAULT); + sendRequest(methodId, args, Transport.DEFAULT); } /** * Called by generated invocation marshaller code; packages up and sends the specified * invocation service request. */ + protected void sendRequest (int methodId, Object[] args, Transport transport) + { + _invdir.sendRequest(_invOid, _invCode, methodId, args, transport); + } + + @Deprecated /** @deprecated use client-argument-free version. */ + protected void sendRequest (Client client, int methodId, Object[] args) + { + sendRequest(client, methodId, args, Transport.DEFAULT); + } + + @Deprecated /** @deprecated use client-argument-free version. */ protected void sendRequest (Client client, int methodId, Object[] args, Transport transport) { client.getInvocationDirector().sendRequest(_invOid, _invCode, methodId, args, transport); @@ -273,4 +300,8 @@ public class InvocationMarshaller /** The invocation service code assigned to this service when it was registered on the * server. */ protected int _invCode; + + /** A reference to the invocation director with whom we interoperate. This is initialized in + * {@link #readObject} when we're read in from the network. */ + protected transient InvocationDirector _invdir; } diff --git a/src/main/java/com/threerings/presents/server/net/PresentsConnection.java b/src/main/java/com/threerings/presents/server/net/PresentsConnection.java index e5fedfe85..d9d84e729 100644 --- a/src/main/java/com/threerings/presents/server/net/PresentsConnection.java +++ b/src/main/java/com/threerings/presents/server/net/PresentsConnection.java @@ -23,6 +23,7 @@ package com.threerings.presents.server.net; import java.io.EOFException; import java.io.IOException; +import java.io.InputStream; import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.nio.channels.DatagramChannel; @@ -203,8 +204,7 @@ public class PresentsConnection extends Connection // we're lazy about creating our input streams because we may be inheriting them from // our authing connection and we don't want to unnecessarily create them in that case if (_fin == null) { - _fin = new FramedInputStream(); - _oin = new ObjectInputStream(_fin); + _oin = createObjectInputStream(_fin = new FramedInputStream()); if (_loader != null) { _oin.setClassLoader(_loader); } @@ -255,6 +255,15 @@ public class PresentsConnection extends Connection return _oin; } + /** + * Creates the object input stream used by this connection to communicate. This may be + * overridden by subclasses to create custom streams. + */ + protected ObjectInputStream createObjectInputStream (InputStream src) + { + return new ObjectInputStream(src); + } + /** * Instructs this connection to inherit its streams from the supplied connection object. This * is called by the connection manager when the time comes to pass streams from the authing diff --git a/src/main/java/com/threerings/presents/server/net/ServerCommunicator.java b/src/main/java/com/threerings/presents/server/net/ServerCommunicator.java index 09bb7b45c..ab37a86c8 100644 --- a/src/main/java/com/threerings/presents/server/net/ServerCommunicator.java +++ b/src/main/java/com/threerings/presents/server/net/ServerCommunicator.java @@ -22,8 +22,11 @@ package com.threerings.presents.server.net; import java.io.IOException; +import java.io.InputStream; +import com.threerings.io.ObjectInputStream; import com.threerings.presents.client.Client; +import com.threerings.presents.client.ClientObjectInputStream; import com.threerings.presents.client.ClientObserver; import com.threerings.presents.client.Communicator; import com.threerings.presents.client.ObserverOps; @@ -88,6 +91,10 @@ public class ServerCommunicator extends Communicator super.networkFailure(ioe); } + @Override protected ObjectInputStream createObjectInputStream (InputStream src) { + return new ClientObjectInputStream(_client, src); + } + @Override protected void closeSocket () { super.closeSocket(); shutdown(); diff --git a/src/main/java/com/threerings/presents/tools/InvocationTask.java b/src/main/java/com/threerings/presents/tools/InvocationTask.java index 8ceab2310..7a1b0852b 100644 --- a/src/main/java/com/threerings/presents/tools/InvocationTask.java +++ b/src/main/java/com/threerings/presents/tools/InvocationTask.java @@ -27,19 +27,21 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.WildcardType; +import java.io.File; import java.util.Arrays; import java.util.List; -import java.io.File; import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.samskivert.util.Logger; import com.samskivert.util.StringUtil; + import com.threerings.presents.annotation.TransportHint; -import com.threerings.presents.net.Transport; +import com.threerings.presents.client.Client; import com.threerings.presents.client.InvocationService.InvocationListener; +import com.threerings.presents.net.Transport; /** * A base Ant task for generating invocation service related marshalling and unmarshalling @@ -81,10 +83,6 @@ public abstract class InvocationTask extends GenTask return _index+1; } - public int getIndexSkipFirst () { - return _index; - } - protected int _index; } @@ -140,18 +138,10 @@ public abstract class InvocationTask extends GenTask } } - public String getArgListSkipFirst () { - return getArgList(true); - } - public String getArgList () { - return getArgList(false); - } - - public String getArgList (boolean skipFirst) { StringBuilder buf = new StringBuilder(); Type[] ptypes = method.getGenericParameterTypes(); - for (int ii = skipFirst ? 1 : 0; ii < ptypes.length; ii++) { + for (int ii = 0; ii < ptypes.length; ii++) { if (buf.length() > 0) { buf.append(", "); } @@ -162,27 +152,19 @@ public abstract class InvocationTask extends GenTask } else { buf.append(simpleName); } - buf.append(" arg").append(skipFirst ? ii : ii+1); + buf.append(" arg").append(ii+1); } return buf.toString(); } - public String getASArgListSkipFirst () { - return getASArgList(true); - } - public String getASArgList () { - return getASArgList(false); - } - - public String getASArgList (boolean skipFirst) { StringBuilder buf = new StringBuilder(); Class[] args = method.getParameterTypes(); - for (int ii = skipFirst ? 1 : 0; ii < args.length; ii++) { + for (int ii = 0; ii < args.length; ii++) { if (buf.length() > 0) { buf.append(", "); } - buf.append("arg").append(skipFirst ? ii : ii+1).append(" :"); + buf.append("arg").append(ii+1).append(" :"); buf.append(GenUtil.simpleASName(args[ii])); } return buf.toString(); @@ -200,18 +182,10 @@ public abstract class InvocationTask extends GenTask return buf.toString(); } - public String getWrappedArgListSkipFirst () { - return getWrappedArgList(true); - } - public String getWrappedArgList () { - return getWrappedArgList(false); - } - - public String getWrappedArgList (boolean skipFirst) { StringBuilder buf = new StringBuilder(); Class[] args = method.getParameterTypes(); - for (int ii = (skipFirst ? 1 : 0); ii < args.length; ii++) { + for (int ii = 0; ii < args.length; ii++) { if (buf.length() > 0) { buf.append(", "); } @@ -220,22 +194,14 @@ public abstract class InvocationTask extends GenTask return buf.toString(); } - public String getASWrappedArgListSkipFirst () { - return getASWrappedArgList(true); - } - public String getASWrappedArgList () { - return getASWrappedArgList(false); - } - - public String getASWrappedArgList (boolean skipFirst) { StringBuilder buf = new StringBuilder(); Class[] args = method.getParameterTypes(); - for (int ii = (skipFirst ? 1 : 0); ii < args.length; ii++) { + for (int ii = 0; ii < args.length; ii++) { if (buf.length() > 0) { buf.append(", "); } - String index = String.valueOf(skipFirst ? ii : (ii+1)); + String index = String.valueOf(ii+1); String arg; if (_ilistener.isAssignableFrom(args[ii])) { arg = GenUtil.boxASArgument(args[ii], "listener" + index); @@ -247,12 +213,8 @@ public abstract class InvocationTask extends GenTask return buf.toString(); } - public boolean hasArgsSkipFirst () { - return (method.getParameterTypes().length > 1); - } - public boolean hasArgs () { - return (method.getParameterTypes().length > 0); + return method.getParameterTypes().length > 0; } public boolean hasParameterizedArgs () { @@ -276,11 +238,11 @@ public abstract class InvocationTask extends GenTask public String getUnwrappedArgList (boolean listenerMode) { StringBuilder buf = new StringBuilder(); Type[] ptypes = method.getGenericParameterTypes(); - for (int ii = (listenerMode ? 0 : 1); ii < ptypes.length; ii++) { + for (int ii = 0; ii < ptypes.length; ii++) { if (buf.length() > 0) { buf.append(", "); } - buf.append(unboxArgument(ptypes[ii], listenerMode ? ii : ii-1, listenerMode)); + buf.append(unboxArgument(ptypes[ii], ii, listenerMode)); } return buf.toString(); } @@ -296,12 +258,12 @@ public abstract class InvocationTask extends GenTask public String getASUnwrappedArgList (boolean listenerMode) { StringBuilder buf = new StringBuilder(); Class[] args = method.getParameterTypes(); - for (int ii = (listenerMode ? 0 : 1); ii < args.length; ii++) { + for (int ii = 0; ii < args.length; ii++) { if (buf.length() > 0) { buf.append(", "); } String arg; - int argidx = listenerMode ? ii : ii-1; + int argidx = ii; if (listenerMode && _ilistener.isAssignableFrom(args[ii])) { arg = "listener" + argidx; } else { @@ -385,8 +347,9 @@ public abstract class InvocationTask extends GenTask @Override public void execute () { - // resolve the InvocationListener class using our classloader + // resolve the InvocationListener and Client classes using our classloader _ilistener = loadClass(InvocationListener.class.getName()); + _iclient = loadClass(Client.class.getName()); super.execute(); } @@ -404,7 +367,11 @@ public abstract class InvocationTask extends GenTask newstr.replace('/', File.separatorChar)); } - /** {@link InvocationListener} resolved with the proper classloader so - * that we can compare it to loaded derived classes. */ + /** {@link InvocationListener} resolved with the proper classloader so that we can compare it + * to loaded derived classes. */ protected Class _ilistener; + + /** {@link Client} resolved with the proper classloader so that we can compare it to loaded + * derived classes. */ + protected Class _iclient; } diff --git a/src/main/resources/com/threerings/presents/tools/dispatcher.tmpl b/src/main/resources/com/threerings/presents/tools/dispatcher.tmpl index 2ffc69d95..b96d3c419 100644 --- a/src/main/resources/com/threerings/presents/tools/dispatcher.tmpl +++ b/src/main/resources/com/threerings/presents/tools/dispatcher.tmpl @@ -36,7 +36,7 @@ public class {{name}}Dispatcher extends InvocationDispatcher<{{name}}Marshaller> {{#methods}} case {{name}}Marshaller.{{code}}: (({{name}}Provider)provider).{{method.name}}( - source{{#hasArgsSkipFirst}}, {{/hasArgsSkipFirst}}{{getUnwrappedArgList}} + source{{#hasArgs}}, {{/hasArgs}}{{getUnwrappedArgList}} ); return; diff --git a/src/main/resources/com/threerings/presents/tools/marshaller.tmpl b/src/main/resources/com/threerings/presents/tools/marshaller.tmpl index d3e349021..d45b2a7b7 100644 --- a/src/main/resources/com/threerings/presents/tools/marshaller.tmpl +++ b/src/main/resources/com/threerings/presents/tools/marshaller.tmpl @@ -72,10 +72,10 @@ public class {{name}}Marshaller extends InvocationMarshaller {{marshaller}} listener{{index}} = new {{marshaller}}(); listener{{index}}.listener = arg{{index}}; {{/listenerArgs}} - sendRequest(arg1, {{code}}, new Object[] { -{{#hasArgsSkipFirst}} - {{getWrappedArgListSkipFirst}} -{{/hasArgsSkipFirst}} + sendRequest({{code}}, new Object[] { +{{#hasArgs}} + {{getWrappedArgList}} +{{/hasArgs}} }{{transport}}); } {{/methods}} diff --git a/src/main/resources/com/threerings/presents/tools/marshaller_as.tmpl b/src/main/resources/com/threerings/presents/tools/marshaller_as.tmpl index 66185877b..feb6de93c 100644 --- a/src/main/resources/com/threerings/presents/tools/marshaller_as.tmpl +++ b/src/main/resources/com/threerings/presents/tools/marshaller_as.tmpl @@ -22,14 +22,14 @@ public class {{name}}Marshaller extends InvocationMarshaller public static const {{code}} :int = {{-index}}; // from interface {{name}}Service - public function {{method.name}} ({{getASArgListSkipFirst}}) :void + public function {{method.name}} ({{getASArgList}}) :void { {{#listenerArgs}} - var listener{{indexSkipFirst}} :{{actionScriptMarshaller}} = new {{actionScriptMarshaller}}(); - listener{{indexSkipFirst}}.listener = arg{{indexSkipFirst}}; + var listener{{index}} :{{actionScriptMarshaller}} = new {{actionScriptMarshaller}}(); + listener{{index}}.listener = arg{{index}}; {{/listenerArgs}} sendRequest({{code}}, [ - {{getASWrappedArgListSkipFirst}} + {{getASWrappedArgList}} ]); } {{/methods}} diff --git a/src/main/resources/com/threerings/presents/tools/provider.tmpl b/src/main/resources/com/threerings/presents/tools/provider.tmpl index c1e34c31d..f3d4e3383 100644 --- a/src/main/resources/com/threerings/presents/tools/provider.tmpl +++ b/src/main/resources/com/threerings/presents/tools/provider.tmpl @@ -19,7 +19,7 @@ public interface {{name}}Provider extends InvocationProvider /** * Handles a {@link {{name}}Service#{{method.name}}} request. */ - void {{method.name}} (ClientObject caller{{#hasArgsSkipFirst}}, {{/hasArgsSkipFirst}}{{getArgListSkipFirst}}){{^listenerArgs.isEmpty}} + void {{method.name}} (ClientObject caller{{#hasArgs}}, {{/hasArgs}}{{getArgList}}){{^listenerArgs.isEmpty}} throws InvocationException{{/listenerArgs.isEmpty}}; {{/methods}} diff --git a/src/main/resources/com/threerings/presents/tools/service_as.tmpl b/src/main/resources/com/threerings/presents/tools/service_as.tmpl index 00ff65b48..b18a26da1 100644 --- a/src/main/resources/com/threerings/presents/tools/service_as.tmpl +++ b/src/main/resources/com/threerings/presents/tools/service_as.tmpl @@ -14,7 +14,7 @@ public interface {{name}}Service extends InvocationService {{/-first}} // from Java interface {{name}}Service - function {{method.name}} ({{getASArgListSkipFirst}}) :void; + function {{method.name}} ({{getASArgList}}) :void; {{/methods}} } }