topcat: fix regression bug (auth failure) in JDBC load dialogue

When I added the TAP load dialogue to the ControlWindow toolbar
just before release of TOPCAT v4.2 (4289ffb), I inadvertently
introduced a bug which meant that the SQL load dialogue doesn't
work any more - authentication fails.  I haven't tracked down
exactly why this happens, but it's related to the fact that
the LoadWindow was being instantiated during ControlWindow
construction (before that change it was not done until the user
hit the Load button).

Modified ControlWindow so that LoadWindow is not constructed
until it's needed, which fixes that bug.  It's possible there
were other unnoticed negative consequences of the earlier change;
this update may fix those too.

Both the problem and the fix are fairly inelegant, because application
startup in topcat (implemented in the very old classes Driver,
ControlWindow, LoadWindow) is messy and badly designed.
Added a note in ControlWindow to this effect.

topcat/src/docs/sun253.xml

@@ -20384,6 +20384,8 @@ introduced since the last version:
         <code>http://reg.g-vo.org/tap</code>,
         which should have good reliability,
         since it can point to different RegTAP services as required.</li>
+    <li>Fix regression bug (introduced at v4.2) which caused authentication
+        to fail when using the SQL Query load dialogue.</li>
     </ul>
     </p></dd>
 

topcat/src/main/uk/ac/starlink/topcat/ControlWindow.java

@@ -145,6 +145,13 @@
  * Main window providing user control of the TOPCAT application.
  * This is a singleton class.
  *
+ * <p><strong>Note:</strong> there is a lot wrong with this class.
+ * It's been here for as long as topcat has (i.e. since before I knew
+ * better), and it does far too much, often in the wrong way.
+ * It would be nice to do something about it one day, but in the
+ * meantime, don't assume that there's a good reason for all the
+ * implementation details that you see here.
+ *
  * @author   Mark Taylor (Starlink)
  * @since    9 Mar 2004
  */
@@ -478,8 +485,15 @@ public void stateChanged( ChangeEvent evt ) {
                                    "five existing tables", 5 ),
         };
 
+        /* Hack.  We want to get an action to launch the TAP load
+         * dialogue here.  One way is to call getLoadWindow() and get it
+         * from there.  However, that instantiates the Load Window too
+         * early, which is not only inefficient (not otherwise required
+         * at startup) but also causes some trouble with JDBC
+         * (the SQL load dialog fails for reasons I haven't identified).
+         * So do it another way. */
         Action tapAct =
-            getLoadWindow().getDialogAction( TopcatTapTableLoadDialog.class );
+           createLoadDialogAction( TopcatTapTableLoadDialog.class );
 
         Transmitter tableTransmitter = communicator_ == null
                                      ? null
@@ -578,7 +592,9 @@ public void mouseClicked( MouseEvent evt ) {
 
         /* Add join/match control buttons to the toolbar. */
         toolBar.add( matchActs_[ 1 ] );
-        toolBar.add( tapAct );
+        if ( tapAct != null ) {
+            toolBar.add( tapAct );
+        }
         toolBar.add( cdsmatchAct_ );
         toolBar.addSeparator();
 
@@ -1930,6 +1946,43 @@ private static String shorten( String label ) {
         return label;
     }
 
+    /**
+     * Returns an action which will launch a load dialogue of a particular
+     * class.  These individual load dialogues are really owned by the
+     * LoadWindow, which doesn't (and shouldn't) exist during
+     * ControlWindow construction, so we have to jump through some hoops.
+     */
+    private Action
+            createLoadDialogAction( final Class<? extends TableLoadDialog>
+                                    tldClazz ) {
+
+        /* Get a dialogue instance for the metadata: name, icon, description.
+         * Throw it away after that.  Not ideal. */
+        TableLoadDialog tld0;
+        try {
+            tld0 = tldClazz.newInstance();
+        }
+        catch ( Throwable e ) {
+            logger_.log( Level.WARNING, "Can't set up TAP action", e );
+            return null;
+        }
+
+        /* Construct and return an action which will lazily acquire the
+         * LoadWindow's copy of the relevant dialogue as required. */
+        return new BasicAction( tld0.getName(), tld0.getIcon(),
+                                tld0.getDescription() ) {
+            Action act_;
+            public void actionPerformed( ActionEvent evt ) {
+                if ( act_ == null ) {
+                    act_ = getLoadWindow().getDialogAction( tldClazz );
+                }
+                if ( act_ != null ) {
+                    act_.actionPerformed( evt );
+                }
+            }
+        };
+    }
+
     /**
      * Actions which correspond to output of a table.
      */

topcat/src/main/uk/ac/starlink/topcat/LoadWindow.java

@@ -284,7 +284,6 @@ public Action getDialogAction( Class tldClazz ) {
                 }
             }
         }
-        assert false;
         return null;
     }