Author: thully
Date: 2012-12-06 13:18:51 -0800 (Thu, 06 Dec 2012)
New Revision: 30908
Modified:
core3/api/trunk/model-api/src/test/java/org/cytoscape/model/AbstractCyTableTest.java
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/CyTableImpl.java
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/LocalTableFacade.java
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/SharedTableFacade.java
Log:
Changes to CyTableImpl/Facades to fix the following issues:
* Resolves #1549 - adding virtual columns to a network which already has
columns (virtual or otherwise) by that name will throw an exception. This also
applies if you are adding to the shared table and any local table in the
hierarchy has an attribute that would clash
* Make sure columns that exist are caught before we start adding to any tables.
This prevents them from being partially added, which was a problem in the
previous code.
* If the shared table doesn't have a particular column, delegate to the local
table in LocalTableFacade for relevant calls (such as renameColumn/deleteColumn)
Also updated tests to reflect the change of expected behavior for duplicate
virtual columns.
Modified:
core3/api/trunk/model-api/src/test/java/org/cytoscape/model/AbstractCyTableTest.java
===================================================================
---
core3/api/trunk/model-api/src/test/java/org/cytoscape/model/AbstractCyTableTest.java
2012-12-06 20:38:35 UTC (rev 30907)
+++
core3/api/trunk/model-api/src/test/java/org/cytoscape/model/AbstractCyTableTest.java
2012-12-06 21:18:51 UTC (rev 30908)
@@ -571,13 +571,13 @@
assertEquals(list, table.getRow(1L).getList(NAME.toUpperCase(),
String.class));
}
- @Test
+ @Test(expected=IllegalArgumentException.class)
public void testAddVirtualColumnIsCaseInsensitive() {
table.createColumn("Col_A", Long.class, false);
table2.createColumn("Col_B", String.class, false);
assertEquals("VirtualCol_A",
table.addVirtualColumn("VirtualCol_A", "COL_B", table2, "col_a", false));
assertEquals("VirtualCol_A",
table.getColumn("VIRTUALCOL_a").getName());
- assertEquals("virtualcol_a-1",
table.addVirtualColumn("virtualcol_a", "col_b", table2, "COL_A", false));
+ table.addVirtualColumn("virtualcol_a", "col_b", table2,
"COL_A", false);
}
@Test
@@ -600,7 +600,7 @@
assertEquals(row.get(primaryKey.getName(),
primaryKey.getType()), 107L);
}
- @Test
+ @Test(expected=IllegalArgumentException.class)
public void testVirtualColumn() {
table.createColumn("x", Long.class, false);
table2.createColumn("s", String.class, false);
@@ -608,8 +608,6 @@
assertEquals("Virtual column type should have been String!",
String.class, table.getColumn("s1").getType());
assertEquals(table.addVirtualColumn("s1", "s", table2, "x",
false), "s1-1");
- assertEquals("Virtual column type should have been String!",
- String.class, table.getColumn("s1-1").getType());
}
@Test
@@ -1023,17 +1021,11 @@
assertEquals(String.class,xcol.getListElementType());
}
- @Test
+ @Test(expected=IllegalArgumentException.class)
public void testVirtualColumnWithDupe() {
table2.createColumn("s", String.class, false);
table.createColumn("s", String.class, false);
table.addVirtualColumns( table2,
table.getPrimaryKey().getName(), false);
-
- CyColumn scol = table.getColumn("s");
- assertFalse( scol.getVirtualColumnInfo().isVirtual() );
-
- CyColumn s1col = table.getColumn("s-1");
- assertTrue( s1col.getVirtualColumnInfo().isVirtual() );
}
@Test
Modified:
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/CyTableImpl.java
===================================================================
---
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/CyTableImpl.java
2012-12-06 20:38:35 UTC (rev 30907)
+++
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/CyTableImpl.java
2012-12-06 21:18:51 UTC (rev 30908)
@@ -463,11 +463,6 @@
if (listElementType == null)
throw new NullPointerException("listElementType
is null");
- if (types.get(columnName) != null)
- throw new IllegalArgumentException("column
already exists with name: '"
- + columnName
+ "' with type: "
- +
types.get(columnName).getType());
-
checkClass(listElementType);
VirtualColumnInfo virtualInfo =
NonVirtualColumnInfo.create(isImmutable);
@@ -1011,6 +1006,11 @@
String targetName = "failed to create column";
synchronized(this) {
+ final String normalizedColName =
normalizeColumnName(virtualColumnName);
+ if (types.containsKey(normalizedColName))
+ throw new IllegalArgumentException("column
already exists with name: '" + virtualColumnName
+ + "' with type: " +
types.get(normalizedColName).getType());
+
final CyColumn sourceColumn =
sourceTable.getColumn(normalizeColumnName(sourceColumnName));
if (sourceColumn == null)
throw new
IllegalArgumentException("\""+sourceColumnName+"\" is not a column in source
table.");
@@ -1026,7 +1026,7 @@
VirtualColumn virtualColumn = new
VirtualColumn((CyTableImpl)sourceTable, sourceColumnName, this,
sourceTable.getPrimaryKey().getName(),
targetJoinKeyName, isImmutable);
- targetName = getUniqueColumnName(virtualColumnName);
+ targetName = virtualColumnName;
final CyColumn targetColumn = new CyColumnImpl(this,
targetName, sourceColumn.getType(),
sourceColumn.getListElementType(), virtualColumn,
@@ -1044,20 +1044,6 @@
return targetName;
}
- private final String getUniqueColumnName(final String preferredName) {
- if (getColumn(preferredName) == null)
- return preferredName;
-
- String newUniqueName;
- int i = 0;
- do {
- ++i;
- newUniqueName = preferredName + "-" + i;
- } while (getColumn(newUniqueName) != null);
-
- return newUniqueName;
- }
-
// Warning: This method is only to be used by CyTableManagerImpl!!!
That's also the reason
// why no ColumnDeletedEvent events are being fired by it!
Also this deletes
// (intentionally!) immutable columns!
@@ -1094,11 +1080,21 @@
+ targetJoinKeyName
+ "\".");
final Collection<CyColumn> columns = sourceTable.getColumns();
+ for(final CyColumn column: columns) {
+ // skip the primary key
+ if (column == sourceTable.getPrimaryKey())
+ continue;
+
+ final String normalizedColName =
normalizeColumnName(column.getName());
+ if (types.containsKey(normalizedColName))
+ throw new IllegalArgumentException("column
already exists with name: '" + column.getName()
+ + "' with type: " +
types.get(normalizedColName).getType());
+ }
for (final CyColumn column : columns) {
- final String columnName = column.getName();
// skip the primary key
- if
(columnName.equalsIgnoreCase(sourceJoinKey.getName()))
+ if (column == sourceTable.getPrimaryKey())
continue;
+ final String columnName = column.getName();
addVirtualColumn(columnName, columnName, sourceTable,
targetJoinKeyName, isImmutable);
}
Modified:
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/LocalTableFacade.java
===================================================================
---
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/LocalTableFacade.java
2012-12-06 20:38:35 UTC (rev 30907)
+++
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/LocalTableFacade.java
2012-12-06 21:18:51 UTC (rev 30908)
@@ -74,7 +74,10 @@
}
public void deleteColumn(String columnName) {
- shared.deleteColumn(columnName);
+ if(shared.getColumn(columnName) != null)
+ shared.deleteColumn(columnName);
+ else
+ local.deleteColumn(columnName);
}
public <T> void createColumn(String columnName, Class<?extends T> type,
boolean isImmutable) {
@@ -96,16 +99,24 @@
}
public String addVirtualColumn(String virtualColumn, String
sourceColumn, CyTable sourceTable, String targetJoinKey, boolean isImmutable) {
- String s = shared.addVirtualColumn(virtualColumn, sourceColumn,
sourceTable, targetJoinKey, isImmutable);
- return s;
+ if(shared.getColumn(targetJoinKey) != null)
+ return shared.addVirtualColumn(virtualColumn,
sourceColumn, sourceTable, targetJoinKey, isImmutable);
+ else
+ return local.addVirtualColumn(virtualColumn,
sourceColumn, sourceTable, targetJoinKey, isImmutable);
}
public void addVirtualColumns(CyTable sourceTable, String
targetJoinKey, boolean isImmutable) {
- shared.addVirtualColumns(sourceTable, targetJoinKey,
isImmutable);
+ if(shared.getColumn(targetJoinKey) != null)
+ shared.addVirtualColumns(sourceTable, targetJoinKey,
isImmutable);
+ else
+ local.addVirtualColumns(sourceTable, targetJoinKey,
isImmutable);
}
@Override
protected void updateColumnName(String oldName, String newName) {
- shared.getColumn(oldName).setName(newName);
+ if(shared.getColumn(oldName) != null)
+ shared.getColumn(oldName).setName(newName);
+ else
+ local.getColumn(oldName).setName(newName);
}
}
Modified:
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/SharedTableFacade.java
===================================================================
---
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/SharedTableFacade.java
2012-12-06 20:38:35 UTC (rev 30907)
+++
core3/impl/trunk/model-impl/impl/src/main/java/org/cytoscape/model/internal/SharedTableFacade.java
2012-12-06 21:18:51 UTC (rev 30908)
@@ -32,6 +32,7 @@
import java.util.ArrayList;
import java.util.List;
+import org.cytoscape.model.CyColumn;
import org.cytoscape.model.CyIdentifiable;
import org.cytoscape.model.CyNetwork;
import org.cytoscape.model.CyNetworkTableManager;
@@ -73,6 +74,22 @@
}
return tables;
}
+
+ private void checkIfAlreadyExists(String columnName) {
+ List<CyTable> tables = localTables();
+ tables.add(0, shared);
+ for(CyTable table: tables) {
+ if (table == null ) {
+ logger.debug("NULL table!");
+ continue;
+ }
+ CyColumn column = table.getColumn(columnName);
+ if(column != null) {
+ throw new IllegalArgumentException("column
already exists with name: '" + columnName
+ + "' with type: " +
column.getType());
+ }
+ };
+ }
public void deleteColumn(String columnName) {
@@ -85,7 +102,7 @@
logger.debug("deleting virtual column: " + columnName +
" from local table: " + local.getTitle());
local.deleteColumn(columnName);
}
- logger.debug("deleting shared olumn: " + columnName + " from
shared table: " + shared.getTitle());
+ logger.debug("deleting shared column: " + columnName + " from
shared table: " + shared.getTitle());
shared.deleteColumn(columnName);
}
@@ -94,6 +111,7 @@
}
public <T> void createColumn(String columnName, Class<?extends T> type,
boolean isImmutable, T defaultValue) {
+ checkIfAlreadyExists(columnName);
logger.debug("adding real column: '" + columnName + "' to
table: " + shared.getTitle());
shared.createColumn(columnName, type, isImmutable,defaultValue);
for ( CyTable local : localTables() ) {
@@ -112,6 +130,7 @@
}
public <T> void createListColumn(String columnName, Class<T>
listElementType, boolean isImmutable, List<T> defaultValue ) {
+ checkIfAlreadyExists(columnName);
logger.debug("adding real List column: '" + columnName + "' to
table: " + shared.getTitle());
shared.createListColumn(columnName, listElementType,
isImmutable, defaultValue);
for ( CyTable local : localTables() ) {
@@ -125,13 +144,18 @@
}
public String addVirtualColumn(String virtualColumn, String
sourceColumn, CyTable sourceTable, String targetJoinKey, boolean isImmutable) {
- virtualColumn = shared.addVirtualColumn(virtualColumn,
sourceColumn, sourceTable, targetJoinKey, isImmutable);
+ checkIfAlreadyExists(virtualColumn);
+ shared.addVirtualColumn(virtualColumn, sourceColumn,
sourceTable, targetJoinKey, isImmutable);
for ( CyTable local : localTables() )
local.addVirtualColumn(virtualColumn, sourceColumn,
sourceTable, targetJoinKey, isImmutable);
return virtualColumn;
}
public void addVirtualColumns(CyTable sourceTable, String
targetJoinKey, boolean isImmutable) {
+ for(CyColumn column: sourceTable.getColumns()) {
+ if (column != sourceTable.getPrimaryKey())
+ checkIfAlreadyExists(column.getName());
+ }
shared.addVirtualColumns(sourceTable, targetJoinKey,
isImmutable);
for ( CyTable local : localTables() )
local.addVirtualColumns(sourceTable, targetJoinKey,
isImmutable);
--
You received this message because you are subscribed to the Google Groups
"cytoscape-cvs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/cytoscape-cvs?hl=en.
