From 08596727e397f5319ba6f8b78c821c360fef6700 Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Sun, 8 May 2016 07:45:20 -0700 Subject: [PATCH 1/8] Make PluginCommand use Reflection more First, look for a 1-arg constructor on the proposed plugin, that takes a HenPlus instance. If it exists, invoke that. If it doesn't, go the usual route and call the no-arg constructor. This way, PluginCommand can try to pass on its reference to the HenPlus instance to the daughter plugin Command, if that Command has a constructor capable of receiving it. This gets around having to change the Command interface with a setHenPlus method. It also uses Reflection to look for an "unregister" method it can call on "plug-out." Note that when this occurs, it swallows any exception. This is by design. The principle we're adhering to is "do no harm." The contract for plugins and the Command interface doesn't require an "unregister" method, so we shouldn't make a federal case out of it if it doesn't exist or we can't call it. --- src/henplus/commands/PluginCommand.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/henplus/commands/PluginCommand.java b/src/henplus/commands/PluginCommand.java index 9ba71c0..d69047c 100644 --- a/src/henplus/commands/PluginCommand.java +++ b/src/henplus/commands/PluginCommand.java @@ -21,6 +21,7 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.PrintWriter; +import java.lang.reflect.InvocationTargetException; import java.util.Iterator; import java.util.Map.Entry; import java.util.SortedMap; @@ -101,7 +102,14 @@ private Command loadPlugin(final String className) throws ClassNotFoundException IllegalAccessException { Command plugin = null; final Class pluginClass = Class.forName(className); - plugin = (Command) pluginClass.newInstance(); + try { + plugin = (Command) pluginClass.getDeclaredConstructor(HenPlus.class).newInstance(_henplus); + _henplus.getDispatcher().register(plugin); + return plugin; + } + catch (Exception e) { + } + plugin = (Command) pluginClass.newInstance(); _henplus.getDispatcher().register(plugin); return plugin; } @@ -154,6 +162,7 @@ public int execute(final SQLSession currentSession, final String cmd, final Stri try { plugin = loadPlugin(pluginClass); } catch (final Exception e) { + e.printStackTrace(System.err); HenPlus.msg().println("couldn't load plugin: " + e.getMessage()); return EXEC_FAILED; } @@ -180,6 +189,11 @@ public int execute(final SQLSession currentSession, final String cmd, final Stri } else { final Command c = _plugins.remove(pluginClass); _henplus.getDispatcher().unregister(c); + try { + Class.forName(pluginClass).getDeclaredMethod("unregister").invoke(c); + } + catch (Exception e) { + } } } return SUCCESS; From ee30907b8a68448b853e932e702f2267559805b1 Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Mon, 22 Feb 2016 07:40:26 -0800 Subject: [PATCH 2/8] Make ResultSetRenderer members protected For sub-classing purposes, it's helpful to make member variables available by setting their access modifiers to 'protected'. I thought about surgically just changing from private-->protected only those members that I needed, but concluded it's cleaner and less confusing just to change them all. Naturally, there's a trade-off of convenience versus coupling here. An alternative approach to leaking the implementation of ResultSetRenderer into its sub-classes would be to provide accessors. I decided that'd be overboard. --- src/henplus/commands/ResultSetRenderer.java | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/henplus/commands/ResultSetRenderer.java b/src/henplus/commands/ResultSetRenderer.java index 33c4146..1c50e05 100644 --- a/src/henplus/commands/ResultSetRenderer.java +++ b/src/henplus/commands/ResultSetRenderer.java @@ -23,17 +23,17 @@ */ public class ResultSetRenderer implements Interruptable { - private final ResultSet _rset; - private final ResultSetMetaData _meta; - private final TableRenderer _table; - private final int _columns; - private final int[] _showColumns; - - private boolean _beyondLimit; - private long _firstRowTime; - private final long _clobLimit = 8192; - private final int _rowLimit; - private volatile boolean _running; + protected final ResultSet _rset; + protected final ResultSetMetaData _meta; + protected final TableRenderer _table; + protected final int _columns; + protected final int[] _showColumns; + + protected boolean _beyondLimit; + protected long _firstRowTime; + protected final long _clobLimit = 8192; + protected final int _rowLimit; + protected volatile boolean _running; public ResultSetRenderer(final ResultSet rset, final String columnDelimiter, final boolean enableHeader, final boolean enableFooter, final int limit, final OutputDevice out, final int[] show) throws SQLException { From 3a6bb80ef63b4200deae1e86c6b98a296c35dfa4 Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Sun, 28 Feb 2016 09:19:09 -0800 Subject: [PATCH 3/8] Make EnumeratedPropertyHolder members protected For sub-classing purposes --- src/henplus/property/EnumeratedPropertyHolder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/henplus/property/EnumeratedPropertyHolder.java b/src/henplus/property/EnumeratedPropertyHolder.java index 962b0f4..22f49ab 100644 --- a/src/henplus/property/EnumeratedPropertyHolder.java +++ b/src/henplus/property/EnumeratedPropertyHolder.java @@ -14,8 +14,8 @@ */ public abstract class EnumeratedPropertyHolder extends PropertyHolder { - private final String[] _values; - private final NameCompleter _completer; + protected final String[] _values; + protected final NameCompleter _completer; /** * create a new EnumeratedPropertyHolder that gets an array of Strings with possible values of this property. From f2cf102b986c26af54a423d4290b5375d82d410a Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Sun, 28 Feb 2016 09:23:07 -0800 Subject: [PATCH 4/8] Make unregisterProperty return old value We want plugins to be able to register new PropertyHolders in place of old PropertyHolders. To do that, they'll have to unregister the old one, first. When that happens, plugins should (if they're well-behaved) remember the old PropertyHolder so they can restore it when they themselves are unregistered. This change sets up this sequence of events by returning the unregistered PropertyHolder to the caller, who can (and should) then squirrel it away for safe-keeping. --- src/henplus/PropertyRegistry.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/henplus/PropertyRegistry.java b/src/henplus/PropertyRegistry.java index 8f38fb6..cc53c12 100644 --- a/src/henplus/PropertyRegistry.java +++ b/src/henplus/PropertyRegistry.java @@ -30,8 +30,8 @@ public void registerProperty(final String name, final PropertyHolder holder) thr _namedProperties.put(name, holder); } - public void unregisterProperty(final String name) { - _namedProperties.remove(name); + public PropertyHolder unregisterProperty(final String name) { + return _namedProperties.remove(name); } /** From 5035276c83b0e310fe65aa02d5ce09af3c104edf Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Tue, 23 Feb 2016 06:30:38 -0800 Subject: [PATCH 5/8] Add a PropertyRegistry accessor to HenPlus Plugin Command objects that want to, say, establish new values for the "output" property, will need access to the "global" property store, held by HenPlus. This accessor method grants that. --- src/henplus/HenPlus.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/henplus/HenPlus.java b/src/henplus/HenPlus.java index 3a9c503..c927273 100644 --- a/src/henplus/HenPlus.java +++ b/src/henplus/HenPlus.java @@ -400,6 +400,14 @@ public String getPartialLine() { return _historyLine.toString() + Readline.getLineBuffer(); } + /** + * Get a reference to this HenPlus instance's internal + * PropertyRegistry. + */ + public PropertyRegistry getRegistry() { + return _henplusProperties; + } + public void run() { String cmdLine = null; String displayPrompt = _prompt; From 35982998391bba719ce48d62bebe03128d866c2d Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Mon, 22 Feb 2016 20:33:26 -0800 Subject: [PATCH 6/8] Add MetaDataPropertyHolder class This is a crucial infrastructure component for supporting more complex behavior around properties, commands, and plugins. Where we're headed is toward enabling plugins (i.e. Command sub-classes) that can manipulate properties. Moreover, to be useful they should be able to manipulate properties that have a richer structure. That's the goal of this particular change. The idea is to create a property that groups together other data. Essentially, it's a "complex" property, with internal structure. There are certainly other approaches for building in rich properties, but I felt this was a modest, incremental one. At the end of the day, the MetaDataPropertyHolder is just an EnumeratedPropertyHolder. It's just that each enumerated property value can have additional data associated with it. This will prove useful in subsequent changes, as we'll see. In particular, where this is useful for the specific use-case that I'm considering is this. I would like to have simple, fairly descriptive property values for different output renderings (e.g., "xml", "json", "text", "csv", etc.), but then each will have to be associated (somehow) with a classname that can be loaded. A plain EnumeratedPropertyHolder could of course handle the strings "xml", "json", etc., of course. But, the question would be, where would we store the corresponding classnames? That's where this new class comes in. It stores them as "meta-data". --- .../property/MetaDataPropertyHolder.java | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/henplus/property/MetaDataPropertyHolder.java diff --git a/src/henplus/property/MetaDataPropertyHolder.java b/src/henplus/property/MetaDataPropertyHolder.java new file mode 100644 index 0000000..6860cad --- /dev/null +++ b/src/henplus/property/MetaDataPropertyHolder.java @@ -0,0 +1,68 @@ +package henplus.property; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +/** + * A PropertyHolder, that can change its values to a fixed set of values. + * + * In addition, for each of those fixed set of values, it can store a + * Map of String-String pairs, as additional meta-data. + */ +public abstract class MetaDataPropertyHolder extends EnumeratedPropertyHolder { + protected final Map> _metaDataValues = new HashMap>(); + + + /** + * Create a new MetaDataPropertyHolder that gets an array of + * Strings with possible values for this property. + */ + public MetaDataPropertyHolder (final String... vals) { + super(vals); + for (String val : vals) { + _metaDataValues.put(val, new HashMap()); + } + if (vals!=null && vals.length>0) { + try { + setValue(vals[0]); + } + catch (Exception e) {} + } + } + + /** + * Same with Collection as input + */ + public MetaDataPropertyHolder (final Collection vals) { + super(vals); + for (String val : vals) { + _metaDataValues.put(val, new HashMap()); + } + } + + public MetaDataPropertyHolder (final Map> vals) { + super(vals.keySet()); + _metaDataValues.putAll(vals); + } + + @Override + public void enumeratedPropertyChanged(final int index, final String value) { + } + + /** + * Get the full Map of meta-data, for the default property. + */ + public Map getMetaData() { + return getMetaData(getValue()); + } + + /** + * Get the full Map of meta-data, for a particular enumerated + * property. + */ + public Map getMetaData(String val) { + return _metaDataValues.get(val); + } +} + From 8453e288dba4487662b803ea7a16f6bd4ded7e69 Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Sun, 28 Feb 2016 18:27:57 -0800 Subject: [PATCH 7/8] Register a MetaDataPropertyHolder as "output" prop We're adding a new system property called "output" and implementing it with a MetaDataPropertyHolder. Recall that a MetaDataPropertyHolder is just an EnumeratedPropertyHolder, except that it can store additional meta-data along with each enumerated value. We use that feature to associate a "renderer-class" meta-data value with the "table" enumerated value (the only and default value, initially), which is set to the stock ResultSetRenderer we all know and love. The reason we're doing this is in anticipation of plugins that can add to the available output types and establish other values for "renderer-class." The current change places HenPlus's original, default renderer-class, ResultSetRenderer, on an equal footing with any potential additional renderers. --- src/henplus/HenPlus.java | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/henplus/HenPlus.java b/src/henplus/HenPlus.java index c927273..98abc54 100644 --- a/src/henplus/HenPlus.java +++ b/src/henplus/HenPlus.java @@ -29,6 +29,7 @@ import henplus.commands.properties.SessionPropertyCommand; import henplus.io.ConfigurationContainer; import henplus.logging.Logger; +import henplus.property.MetaDataPropertyHolder; import henplus.util.StringUtil; import java.io.BufferedReader; @@ -39,6 +40,7 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.io.UnsupportedEncodingException; +import java.util.Collection; import java.util.Iterator; import java.util.Map; @@ -145,6 +147,39 @@ public void readConfiguration(final InputStream in) throws Exception { setDefaultPrompt(); } + public static class OutputTypePropertyHolder extends MetaDataPropertyHolder { + public OutputTypePropertyHolder (final String... vals) { + super(vals); + } + + public OutputTypePropertyHolder (final Collection vals) { + super(vals); + } + + public OutputTypePropertyHolder (final Map> vals) { + super(vals.keySet()); + } + @Override + public void enumeratedPropertyChanged(final int index, final String value) { + } + @Override + public String getDefaultValue() { + return "table"; + } + @Override + public String getShortDescription () { + return "set the output type"; + } + @Override + public String getLongDescription () { + return "Set the output type"; + } + @Override + public Map getMetaData() { + return getMetaData(getValue()); + } + } + public void initializeCommands(final String[] argv) { _henplusProperties = new PropertyRegistry(); _henplusProperties.registerProperty("comments-remove", _commandSeparator.getRemoveCommentsProperty()); @@ -156,6 +191,9 @@ public void initializeCommands(final String[] argv) { _dispatcher = new CommandDispatcher(_settingStore); _objectLister = new ListUserObjectsCommand(this); _henplusProperties.registerProperty("echo-commands", new EchoCommandProperty(_dispatcher)); + MetaDataPropertyHolder renderers = new OutputTypePropertyHolder("table"); + renderers.getMetaData().put("renderer-class", "henplus.commands.ResultSetRenderer"); + _henplusProperties.registerProperty("output", renderers); _dispatcher.register(new HelpCommand()); From d1b538d8b8aa6ca6b7b46ab180afab41b78f5973 Mon Sep 17 00:00:00 2001 From: "David A. Ventimiglia" Date: Sun, 28 Feb 2016 18:28:31 -0800 Subject: [PATCH 8/8] Make SQLCommand load ResultSetRenderer dynamically Now that the stock ResultSetRenderer has been set as the "renderer-class" meta-data element for the "table" output type, we can switch to a Dependency-Injection model for creating the ResultSetRenderer (sub-class). Rather than hard-code it, it now uses reflection to instantiate a sub-type, which is determined by the "renderer-class" meta-data value. --- src/henplus/commands/SQLCommand.java | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/henplus/commands/SQLCommand.java b/src/henplus/commands/SQLCommand.java index d2f350d..24ccea6 100644 --- a/src/henplus/commands/SQLCommand.java +++ b/src/henplus/commands/SQLCommand.java @@ -8,12 +8,14 @@ import henplus.AbstractCommand; import henplus.CommandDispatcher; import henplus.HenPlus; +import henplus.OutputDevice; import henplus.PropertyRegistry; import henplus.SQLSession; import henplus.SigIntHandler; import henplus.logging.Logger; import henplus.property.BooleanPropertyHolder; import henplus.property.PropertyHolder; +import henplus.property.MetaDataPropertyHolder; import henplus.view.util.CancelWriter; import henplus.view.util.NameCompleter; @@ -58,6 +60,7 @@ public String[] getCommandList() { private boolean _showFooter; private volatile boolean _running; private StatementCanceller _statementCanceller; + private PropertyRegistry _registry; protected SQLCommand(final ListUserObjectsCommand tc) { _columnDelimiter = "|"; @@ -73,6 +76,7 @@ public SQLCommand(final ListUserObjectsCommand tc, final PropertyRegistry regist _rowLimit = 2000; _showHeader = true; _showFooter = true; + _registry = registry; registry.registerProperty("column-delimiter", new SQLColumnDelimiterProperty()); registry.registerProperty("sql-result-limit", new RowLimitProperty()); registry.registerProperty("sql-result-showheader", new ShowHeaderProperty()); @@ -83,6 +87,10 @@ public SQLCommand(final ListUserObjectsCommand tc, final PropertyRegistry regist new Thread(_longRunningDisplay).start(); } + public PropertyRegistry getRegistry() { + return _registry; + } + /** * don't show the commands available in the toplevel command completion list .. */ @@ -244,8 +252,20 @@ public int execute(final SQLSession session, final String cmd, final String para do { rset = _stmt.getResultSet(); ResultSetRenderer renderer; - renderer = new ResultSetRenderer(rset, getColumnDelimiter(), isShowHeader(), isShowFooter(), getRowLimit(), - HenPlus.out()); + renderer = + (ResultSetRenderer) + Class + .forName(((MetaDataPropertyHolder)getRegistry().getPropertyMap().get("output")) + .getMetaData() + .get("renderer-class")) + .getDeclaredConstructor(ResultSet.class, + String.class, + boolean.class, + boolean.class, + int.class, + OutputDevice.class) + .newInstance(rset, getColumnDelimiter(), isShowHeader(), isShowFooter(), getRowLimit(), + HenPlus.out()); SigIntHandler.getInstance().pushInterruptable(renderer); final int rows = renderer.execute(); SigIntHandler.getInstance().popInterruptable();