Author: jfuerth
Date: Wed Nov 5 15:42:42 2008
New Revision: 2815
Modified:
trunk/regress/ca/sqlpower/architect/swingui/TestSwingUIProject.java
trunk/src/ca/sqlpower/architect/CoreProject.java
trunk/src/ca/sqlpower/architect/SQLIndex.java
Log:
Fixed bug 1662.
This problem turned out to be caused by a misguided attempt at
null "safety." The SQLIndex.setParent() method would silently skip adding
the column folder listener responsible for cleanup if its new parent folder
was unparented. This situation was true during project load, so all non-PK
indexes loaded from a project file exhibited this problem, but the "fresh"
ones already being tested in the test suite didn't have the problem.
The SQLIndex.setParent() method now throws an exception when the new parent
folder has no parent table, and the load routine has been fixed up so the
folder is parented before the indexes are added.
Modified:
trunk/regress/ca/sqlpower/architect/swingui/TestSwingUIProject.java
==============================================================================
--- trunk/regress/ca/sqlpower/architect/swingui/TestSwingUIProject.java
(original)
+++ trunk/regress/ca/sqlpower/architect/swingui/TestSwingUIProject.java Wed
Nov 5 15:42:42 2008
@@ -28,6 +28,7 @@
import java.io.PrintWriter;
import java.sql.Connection;
import java.sql.Types;
+import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -1219,5 +1220,40 @@
SQLObjectRoot rootObject = session.getRootObject();
SQLDatabase ppdb = (SQLDatabase) rootObject.getChild(0);
assertTrue(ppdb.isPlayPenDatabase());
+ }
+
+ /**
+ * Regression test for bug 1662: after loading a project, all the
indexes
+ * have to be listening to the columns folder of the table they belong
to.
+ */
+ public void testIndexListensToColumnsAfterLoad() throws Exception {
+ testLoad();
+ SQLDatabase ppdb = session.getTargetDatabase();
+ SQLTable t = ppdb.getTableByName("mm_project");
+
+ // have to do the same check for every index, because (for example)
+ // the PK index didn't exhibit this problem
+ for (SQLIndex idx : new ArrayList<SQLIndex>(t.getIndices())) {
+
+ assertTrue(
+ "didn't expect this index to have no columns!",
+ idx.getChildCount() > 0);
+
+ List<SQLIndex.Column> indexCols = idx.getChildren();
+ List<SQLColumn> colsToRemove = new ArrayList<SQLColumn>();
+ for (SQLIndex.Column indexCol : indexCols) {
+ colsToRemove.add(indexCol.getColumn());
+ }
+
+ for (SQLColumn col : colsToRemove) {
+ t.removeColumn(col);
+ }
+
+ // prove the listener was hooked up
+ assertTrue(
+ "Index " + idx + " wasn't listening to columns
folder!",
+ idx.getChildCount() == 0);
+ assertFalse(t.getIndices().contains(idx));
+ }
}
}
Modified: trunk/src/ca/sqlpower/architect/CoreProject.java
==============================================================================
--- trunk/src/ca/sqlpower/architect/CoreProject.java (original)
+++ trunk/src/ca/sqlpower/architect/CoreProject.java Wed Nov 5 15:42:42
2008
@@ -329,7 +329,7 @@
SQLFolderFactory folderFactory = new SQLFolderFactory();
d.addFactoryCreate("*/folder", folderFactory);
d.addSetProperties("*/folder");
- d.addSetNext("*/folder", "addChild");
+ // the factory adds the folder to the table upon creation
SQLColumnFactory columnFactory = new SQLColumnFactory();
d.addFactoryCreate("*/column", columnFactory);
@@ -518,6 +518,14 @@
}
/**
+ * The table most recently loaded from the project file. The
SQLFolderFactory
+ * has to know which table it's creating a folder for, because it has
to add
+ * the folder upon creation instead of waiting for the digester to do
it at the
+ * end of the enclosing table element.
+ */
+ private SQLTable currentTable;
+
+ /**
* Creates a SQLTable instance and adds it to the objectIdMap.
*/
private class SQLTableFactory extends AbstractObjectCreationFactory {
@@ -543,6 +551,7 @@
}
}
+ currentTable = tab;
return tab;
}
}
@@ -569,7 +578,17 @@
+typeStr+"\"");
}
}
- return new SQLTable.Folder(type, true);
+
+ // have to add folders right away because some stuff (for
example, the SQLIndex constructor)
+ // requires that the folder's parent table is already linked
up. It's done here because
+ // the Digester is incapable of doing this up front (see
javadoc for CallMethodRule)
+ Folder f = new SQLTable.Folder(type, true);
+ try {
+ currentTable.addChild(f);
+ } catch (ArchitectException ex) {
+ throw new ArchitectRuntimeException(ex);
+ }
+ return f;
}
}
Modified: trunk/src/ca/sqlpower/architect/SQLIndex.java
==============================================================================
--- trunk/src/ca/sqlpower/architect/SQLIndex.java (original)
+++ trunk/src/ca/sqlpower/architect/SQLIndex.java Wed Nov 5 15:42:42 2008
@@ -454,17 +454,31 @@
}
/**
- * Updates this index's parent reference.
- *
- * @param parent must be a SQLTable.Folder instance.
+ * Updates this index's parent reference, and attaches a listener to
the
+ * columns folder of the new parent's parent table.
+ *
+ * @param parent
+ * The new parent. Must be null or a SQLTable.Folder
instance. If
+ * it's a folder, it must already have a parent table.
+ * @throws IllegalStateException
+ * if the given parent is non-null and does not itself
have a
+ * parent table.
*/
@Override
protected void setParent(SQLObject parent) {
+
+ if (this.parent != null) {
+
getParentTable().getColumnsFolder().removeSQLObjectListener(removeColumnListener);
+ }
+
this.parent = (Folder<SQLIndex>) parent;
- if (this.parent != null && this.parent.getParent() != null) {
-
this.parent.getParent().getColumnsFolder().addSQLObjectListener(removeColumnListener);
+
+ if (this.parent != null) {
+ if (getParentTable() == null) {
+ throw new IllegalStateException("Index folder is
unparented!");
+ }
+
getParentTable().getColumnsFolder().addSQLObjectListener(removeColumnListener);
}
-
}
/**
