Author: thomasobrien95
Date: Fri Dec 5 12:07:03 2008
New Revision: 2866
Added:
trunk/regress/ca/sqlpower/architect/swingui/IndexColumnTableTest.java
Modified:
trunk/src/ca/sqlpower/architect/SQLIndex.java
trunk/src/ca/sqlpower/architect/swingui/IndexColumnTable.java
trunk/src/ca/sqlpower/architect/swingui/action/DeleteSelectedAction.java
trunk/src/ca/sqlpower/architect/swingui/dbtree/DBTreeCellRenderer.java
Log:
This is a fix for bug 1526. Expanding an index in the DB tree was
throwing a NPE as the SQLColumn in a Column was null due to the
Column being constructed from a function. Null checks, and a fix to
the index properties window removed the NPE and allows the index to
be saved with an existing column index from a function.
A test case has also been added.
Added: trunk/regress/ca/sqlpower/architect/swingui/IndexColumnTableTest.java
==============================================================================
--- (empty file)
+++ trunk/regress/ca/sqlpower/architect/swingui/IndexColumnTableTest.java
Fri Dec 5 12:07:03 2008
@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2008, SQL Power Group Inc.
+ *
+ * This file is part of Power*Architect.
+ *
+ * Power*Architect is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Power*Architect 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package ca.sqlpower.architect.swingui;
+
+import junit.framework.TestCase;
+import ca.sqlpower.architect.SQLIndex;
+import ca.sqlpower.architect.SQLTable;
+import ca.sqlpower.architect.SQLIndex.AscendDescend;
+import ca.sqlpower.architect.SQLIndex.Column;
+import
ca.sqlpower.architect.swingui.IndexColumnTable.IndexColumnTableModel;
+
+public class IndexColumnTableTest extends TestCase {
+
+ /**
+ * This will test the PopulateModel method on the IndexColumnTableModel
+ * to verify the table does not throw an exception when an index is
added
+ * that has a column with no SQLColumn. This is related to bug 1526.
+ */
+ public void testTableModelPopulateAllowsNulls() throws Exception {
+ SQLTable table = new SQLTable();
+ table.initFolders(true);
+ SQLIndex index = new SQLIndex();
+ Column col = index.new Column("TestCol",
AscendDescend.UNSPECIFIED);
+ index.addChild(col);
+ table.addIndex(index);
+ SQLIndex copyIndex = new SQLIndex(index);
+ IndexColumnTable ict = new IndexColumnTable(table, copyIndex,
index);
+
+ IndexColumnTableModel ictm = ict.new
IndexColumnTableModel(copyIndex, table, index);
+
+ assertEquals(1, ictm.getRowCount());
+ assertEquals(1, index.getChildCount());
+
+ System.out.println(ictm.getRowList());
+
+ ict.finalizeIndex();
+
+ System.out.println(ictm.getRowList());
+
+ assertEquals(1, index.getChildCount());
+
+ //Assert the column not the null SQLColumn in the column to come
back.
+ assertEquals(col, ictm.getValueAt(0, 1));
+ }
+}
Modified: trunk/src/ca/sqlpower/architect/SQLIndex.java
==============================================================================
--- trunk/src/ca/sqlpower/architect/SQLIndex.java (original)
+++ trunk/src/ca/sqlpower/architect/SQLIndex.java Fri Dec 5 12:07:03 2008
@@ -201,6 +201,10 @@
}
}
+ /**
+ * NOTE: This column can be null if the column it represents is an
expression
+ * and not a basic column.
+ */
public SQLColumn getColumn() {
return column;
}
@@ -490,7 +494,7 @@
try {
for (int i = 0; i < e.getChildren().length; i++) {
for (int j = this.getChildCount() - 1; j >= 0; j--) {
- if
(getChild(j).getColumn().equals(e.getChildren()[i])) {
+ if (getChild(j).getColumn() != null &&
getChild(j).getColumn().equals(e.getChildren()[i])) {
removeChild(j);
}
}
@@ -760,8 +764,16 @@
return getName();
}
+ /**
+ * Adds a column to the index. If col1 is null a NPE will be thrown.
+ */
public void addIndexColumn(SQLColumn col1, AscendDescend aOrD) throws
ArchitectException {
Column col = new Column(col1, aOrD);
+ addChild(col);
+ }
+
+ public void addIndexColumn(String colName, AscendDescend aOrD) throws
ArchitectException {
+ Column col = new Column(colName, aOrD);
addChild(col);
}
Modified: trunk/src/ca/sqlpower/architect/swingui/IndexColumnTable.java
==============================================================================
--- trunk/src/ca/sqlpower/architect/swingui/IndexColumnTable.java
(original)
+++ trunk/src/ca/sqlpower/architect/swingui/IndexColumnTable.java Fri Dec
5 12:07:03 2008
@@ -45,6 +45,8 @@
import javax.swing.table.TableCellRenderer;
import javax.swing.table.TableColumn;
+import org.apache.log4j.Logger;
+
import ca.sqlpower.architect.ArchitectException;
import ca.sqlpower.architect.ArchitectRuntimeException;
import ca.sqlpower.architect.SQLColumn;
@@ -64,8 +66,13 @@
* index. This table will be used by the IndexEditPanel class.
*/
public class IndexColumnTable {
+
+ private static final Logger logger =
Logger.getLogger(IndexColumnTable.class);
- private class IndexColumnTableModel extends AbstractTableModel
implements CleanupTableModel, SQLObjectListener {
+ /**
+ * This is package private for testing purposes.
+ */
+ class IndexColumnTableModel extends AbstractTableModel implements
CleanupTableModel, SQLObjectListener {
/**
* This List contains all of the Row objects in the table. Refer
to the
@@ -199,7 +206,11 @@
private void populateModel() {
try {
for (Column indexCol : index.getChildren()) {
- rowList.add(new Row(true, indexCol.getColumn(),
indexCol.getAscendingOrDescending()));
+ if (indexCol.getColumn() != null) {
+ rowList.add(new Row(true, indexCol.getColumn(),
indexCol.getAscendingOrDescending()));
+ } else {
+ rowList.add(new Row(true, indexCol,
indexCol.getAscendingOrDescending()));
+ }
}
for (Object so : columnsFolder.getChildren()) {
SQLColumn col = (SQLColumn) so;
@@ -288,16 +299,26 @@
public Object getValueAt(int row, int col) {
if (col == 0) return
Boolean.valueOf(rowList.get(row).isEnabled());
- else if (col == 1) return rowList.get(row).getSQLColumn();
+ else if (col == 1 && rowList.get(row).getSQLColumn() != null)
return rowList.get(row).getSQLColumn();
+ else if (col == 1) return rowList.get(row).getColumn();
else if (col == 2) return rowList.get(row).getOrder();
else throw new ArchitectRuntimeException(new
ArchitectException("This table only has 3 columns.")); //$NON-NLS-1$
}
public void setValueAt(Object value, int row, int col) {
- if (col == 0) rowList.get(row).setEnabled(((Boolean)
value).booleanValue());
- else if (col == 1) rowList.get(row).setSQLColumn((SQLColumn)
value);
- else if (col == 2) rowList.get(row).setOrder((AscendDescend)
value);
- else throw new ArchitectRuntimeException(new
ArchitectException("This table only has 3 columns.")); //$NON-NLS-1$
+ if (col == 0) {
+ rowList.get(row).setEnabled(((Boolean)
value).booleanValue());
+ } else if (col == 1) {
+ if (value instanceof SQLColumn) {
+ rowList.get(row).setSQLColumn((SQLColumn) value);
+ } else {
+ rowList.get(row).setColumn((Column) value);
+ }
+ } else if (col == 2) {
+ rowList.get(row).setOrder((AscendDescend) value);
+ } else {
+ throw new ArchitectRuntimeException(new
ArchitectException("This table only has 3 columns.")); //$NON-NLS-1$
+ }
fireTableCellUpdated(row, col);
}
@@ -359,21 +380,38 @@
/**
* This is the SQLCOlumn in the row
*/
- private SQLColumn column;
+ private SQLColumn sqlColumn;
/**
* This is the order of the SQLColumn
*/
private AscendDescend order;
+
+ /**
+ * This is a Column in the row for Columns that have no
+ * SQLColumn. Columns have no SQLColumn when an index
+ * is created from a function or possibly other ways.
+ */
+ private Column column;
public Row(boolean enabled, SQLColumn column, AscendDescend order)
{
this.enabled = enabled;
+ this.sqlColumn = column;
+ this.order = order;
+ }
+
+ public Row(boolean enabled, Column column, AscendDescend order) {
+ this.enabled = enabled;
this.column = column;
this.order = order;
}
public String toString() {
- return new String ("Enabled: " +enabled + " Col: " +
column.getName()); //$NON-NLS-1$ //$NON-NLS-2$
+ if (sqlColumn != null) {
+ return new String ("Enabled: " +enabled + " Col: " +
sqlColumn.getName()); //$NON-NLS-1$ //$NON-NLS-2$
+ } else {
+ return new String ("Enabled: " +enabled + " Col: " +
column.getName()); //$NON-NLS-1$ //$NON-NLS-2$
+ }
}
public void setEnabled(boolean enabled) {
@@ -381,7 +419,7 @@
}
public void setSQLColumn(SQLColumn column) {
- this.column = column;
+ this.sqlColumn = column;
}
public void setOrder(AscendDescend order) {
@@ -393,13 +431,21 @@
}
public SQLColumn getSQLColumn() {
- return this.column;
+ return this.sqlColumn;
}
public AscendDescend getOrder() {
return this.order;
}
+ public void setColumn(Column column) {
+ this.column = column;
+ }
+
+ public Column getColumn() {
+ return column;
+ }
+
}
public static final String IN_INDEX =
Messages.getString("IndexColumnTable.indexTableColumnName"); //$NON-NLS-1$
@@ -455,7 +501,12 @@
}
for(Row r : model.getRowList()) {
if (r.isEnabled()){
- index.addIndexColumn(r.getSQLColumn(), r.getOrder());
+ if (r.getSQLColumn() != null) {
+ index.addIndexColumn(r.getSQLColumn(),
r.getOrder());
+ } else {
+ logger.debug("Adding index column with no
SQLColumn. Column name is " + r.getColumn().getName());
+ index.addIndexColumn(r.getColumn().getName(),
r.getOrder());
+ }
}
}
Modified:
trunk/src/ca/sqlpower/architect/swingui/action/DeleteSelectedAction.java
==============================================================================
---
trunk/src/ca/sqlpower/architect/swingui/action/DeleteSelectedAction.java
(original)
+++
trunk/src/ca/sqlpower/architect/swingui/action/DeleteSelectedAction.java
Fri Dec 5 12:07:03 2008
@@ -141,7 +141,9 @@
cols.add(col.getColumn());
}
for (SQLColumn col : cols) {
- col.setPrimaryKeySeq(null);
+ if (col != null) {
+ col.setPrimaryKeySeq(null);
+ }
}
} else {
o.getParent().removeChild(o);
Modified:
trunk/src/ca/sqlpower/architect/swingui/dbtree/DBTreeCellRenderer.java
==============================================================================
--- trunk/src/ca/sqlpower/architect/swingui/dbtree/DBTreeCellRenderer.java
(original)
+++ trunk/src/ca/sqlpower/architect/swingui/dbtree/DBTreeCellRenderer.java
Fri Dec 5 12:07:03 2008
@@ -29,6 +29,8 @@
import javax.swing.JTree;
import javax.swing.tree.DefaultTreeCellRenderer;
+import org.apache.log4j.Logger;
+
import ca.sqlpower.architect.SQLCatalog;
import ca.sqlpower.architect.SQLColumn;
import ca.sqlpower.architect.SQLDatabase;
@@ -51,6 +53,8 @@
*/
public class DBTreeCellRenderer extends DefaultTreeCellRenderer {
+ private static final Logger logger =
Logger.getLogger(DBTreeCellRenderer.class);
+
public static final ImageIcon DB_ICON = new
ImageIcon(DBTreeCellRenderer.class.getResource("icons/Database16.png"));
public static final ImageIcon TARGET_DB_ICON = new
ImageIcon(DBTreeCellRenderer.class.getResource("icons/Database_target16.png"));
public static final ImageIcon CATALOG_ICON = new
ImageIcon(DBTreeCellRenderer.class.getResource("icons/Catalog16.png"));
@@ -131,7 +135,11 @@
tagColumn((SQLColumn)value);
setIcon(COLUMN_ICON);
} else if (value instanceof Column) {
- tagColumn(((Column)value).getColumn());
+ Column col = (Column) value;
+ logger.debug("Column has properties " + col);
+ if (col.getColumn() != null) {
+ tagColumn((col).getColumn());
+ }
setIcon(COLUMN_ICON);
} else {
setIcon(null);
