Revision: 3660
Author: [email protected]
Date: Wed Jun 30 12:59:24 2010
Log: NEW - bug 2458: Create Critic Manager
http://trillian.sqlpower.ca/bugzilla/show_bug.cgi?id=2458
Added in a critic to check for duplicate names in support of replacing the
quick fix system.
Added in a start and end method to the critics to allow critics to keep
state of each run and to know when to setup and tear down its state.
Removed the getWarnings method from the DDLGenerator interface. This is the
first step of cleaning up the current quick fix system but more is to come.
http://code.google.com/p/power-architect/source/detail?r=3660
Added:
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/DuplicateNameCritic.java
Modified:
/trunk/regress/ca/sqlpower/architect/ddl/GenericDDLGeneratorTest.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/DDLGenerator.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/GenericDDLGenerator.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/LiquibaseDDLGenerator.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/OracleDDLGenerator.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/PostgresDDLGenerator.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/Critic.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/CriticAndSettings.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/CriticManager.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/Criticizer.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/AlphaNumericNameCritic.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/OraclePhysicalNameCritic.java
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/PhysicalNameCritic.java
=======================================
--- /dev/null
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/DuplicateNameCritic.java
Wed Jun 30 12:59:24 2010
@@ -0,0 +1,138 @@
+/*
+ * Copyright (c) 2010, SQL Power Group Inc.
+ *
+ * This file is part of SQL Power Architect.
+ *
+ * SQL 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.
+ *
+ * SQL 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.ddl.critic.impl;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import ca.sqlpower.architect.ddl.critic.CriticAndSettings;
+import ca.sqlpower.architect.ddl.critic.Criticism;
+import ca.sqlpower.architect.ddl.critic.QuickFix;
+import ca.sqlpower.sqlobject.SQLColumn;
+import ca.sqlpower.sqlobject.SQLIndex;
+import ca.sqlpower.sqlobject.SQLObject;
+import ca.sqlpower.sqlobject.SQLRelationship;
+import ca.sqlpower.sqlobject.SQLTable;
+
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
+
+/**
+ * This critic will create a warning for objects that have the same name
if they
+ * are of conflicting types. This means some objects, like columns in
different
+ * tables, can have the same name but a sequence cannot have the same name
as a
+ * table because they are at the same level in a database.
+ */
+public class DuplicateNameCritic extends CriticAndSettings {
+
+ /**
+ * Stores all of the top level target database objects by name for the
+ * current run of the critics. The definition of top level is any
object
+ * that is in the target database that would be at the same level in a
+ * database when written out to a DDL script. This includes tables,
+ * relationships, indices, and sequences.
+ * <p>
+ * Note: sequences are represented by columns in this map as there is
no
+ * sequence object.
+ */
+ private Multimap<String, SQLObject> topLevelPhysicalNameMap =
ArrayListMultimap.create();
+
+ /**
+ * Maps each parent table to the columns in the table that we have
+ * critiqued. This way we can find if there are columns with matching
column
+ * names.
+ */
+ private Multimap<SQLTable, SQLColumn> columnPhysicalNameMap =
ArrayListMultimap.create();
+
+ public DuplicateNameCritic() {
+ super(StarterPlatformTypes.GENERIC.getName(), "Error on objects
with the same name.");
+ }
+
+ @Override
+ public void start() {
+ super.start();
+ topLevelPhysicalNameMap.clear();
+ columnPhysicalNameMap.clear();
+ }
+
+ @Override
+ public void end() {
+ super.end();
+ topLevelPhysicalNameMap.clear();
+ columnPhysicalNameMap.clear();
+ }
+
+ public List<Criticism> criticize(Object subject) {
+ if (!(subject instanceof SQLObject)) return
Collections.emptyList();
+
+ List<Criticism> criticisms = new ArrayList<Criticism>();
+ if (subject instanceof SQLColumn) {
+ final SQLColumn col = (SQLColumn) subject;
+ SQLTable parent = col.getParent();
+
+ int count = 0;
+ for (SQLColumn otherCol : columnPhysicalNameMap.get(parent)) {
+ if (col.getPhysicalName().equals(otherCol)) {
+ count++;
+ }
+ }
+ if (count > 0) {
+ final String newPhysicalName = col.getPhysicalName() + "_"
+ count;
+ criticisms.add(new Criticism(subject,
+ "Duplicate physical name \"" +
col.getPhysicalName() + "\"", this,
+ new QuickFix("Replace physical name with " +
newPhysicalName) {
+ @Override
+ public void apply() {
+ col.setPhysicalName(newPhysicalName);
+ }
+ }));
+ }
+ columnPhysicalNameMap.put(parent, col);
+ }
+ if (subject instanceof SQLTable || subject instanceof
SQLRelationship ||
+ subject instanceof SQLIndex || subject instanceof
SQLColumn) {
+ final SQLObject obj = (SQLObject) subject;
+ String physicalName = obj.getPhysicalName();
+ if (obj instanceof SQLColumn) {
+ physicalName = ((SQLColumn)
obj).getAutoIncrementSequenceName();
+ }
+ Collection<SQLObject> sameNameObjects =
topLevelPhysicalNameMap.get(physicalName);
+ if (!sameNameObjects.isEmpty()) {
+ final String newPhysicalName = physicalName + "_" +
sameNameObjects.size();
+ criticisms.add(new Criticism(subject, "Duplicate physical
name \"" + physicalName + "\"", this,
+ new QuickFix("Replace physical name with " +
newPhysicalName) {
+ @Override
+ public void apply() {
+ if (obj instanceof SQLColumn) {
+ ((SQLColumn)
obj).setAutoIncrementSequenceName(newPhysicalName);
+ } else {
+ obj.setPhysicalName(newPhysicalName);
+ }
+ }
+ }));
+ }
+ topLevelPhysicalNameMap.put(physicalName, obj);
+ }
+ return criticisms;
+ }
+
+}
=======================================
--- /trunk/regress/ca/sqlpower/architect/ddl/GenericDDLGeneratorTest.java
Thu May 27 07:42:00 2010
+++ /trunk/regress/ca/sqlpower/architect/ddl/GenericDDLGeneratorTest.java
Wed Jun 30 12:59:24 2010
@@ -19,35 +19,16 @@
package ca.sqlpower.architect.ddl;
-import ca.sqlpower.sqlobject.SQLColumn;
+import java.sql.Types;
+import java.util.List;
+
import junit.framework.TestCase;
-import ca.sqlpower.sqlobject.SQLRelationship;
+import ca.sqlpower.sqlobject.SQLColumn;
import ca.sqlpower.sqlobject.SQLTable;
import ca.sqlpower.sqlobject.SQLType;
import
ca.sqlpower.sqlobject.SQLTypePhysicalPropertiesProvider.PropertyType;
-import java.sql.Types;
-import java.util.List;
-
public class GenericDDLGeneratorTest extends TestCase {
-
- /**
- * Regression testing for bug 1354. If a relationship is forward
engineered
- * but has no columns then it should be skipped.
- */
- public void testSkipRelationWithNoColumns() throws Exception {
- GenericDDLGenerator ddl = new GenericDDLGenerator();
- SQLRelationship r = new SQLRelationship();
- r.setName("Test Relation");
- SQLTable pkTable = new SQLTable();
- pkTable.initFolders(true);
- SQLTable fkTable = new SQLTable();
- fkTable.initFolders(true);
- r.attachRelationship(pkTable, fkTable, true);
- ddl.addRelationship(r);
-
- assertEquals(1, ddl.getWarnings().size());
- }
public void testGenerateComment() throws Exception {
GenericDDLGenerator ddl = new GenericDDLGenerator();
=======================================
--- /trunk/src/main/java/ca/sqlpower/architect/ddl/DDLGenerator.java Tue
May 18 11:21:10 2010
+++ /trunk/src/main/java/ca/sqlpower/architect/ddl/DDLGenerator.java Wed
Jun 30 12:59:24 2010
@@ -261,11 +261,6 @@
*/
public void setTypeMap(Map<Integer, GenericTypeDescriptor> argTypeMap);
- /**
- * Returns {...@link #warnings}.
- */
- public List<DDLWarning> getWarnings();
-
/**
* See {...@link #targetCatalog}.
*
=======================================
--- /trunk/src/main/java/ca/sqlpower/architect/ddl/GenericDDLGenerator.java
Mon Jun 14 11:54:52 2010
+++ /trunk/src/main/java/ca/sqlpower/architect/ddl/GenericDDLGenerator.java
Wed Jun 30 12:59:24 2010
@@ -43,7 +43,6 @@
import ca.sqlpower.sqlobject.SQLObject;
import ca.sqlpower.sqlobject.SQLObjectException;
import ca.sqlpower.sqlobject.SQLRelationship;
-import ca.sqlpower.sqlobject.SQLSequence;
import ca.sqlpower.sqlobject.SQLTable;
import ca.sqlpower.sqlobject.SQLType;
import ca.sqlpower.sqlobject.SQLTypePhysicalPropertiesProvider;
@@ -117,16 +116,6 @@
*/
protected Map<String, SQLObject> topLevelNames;
- /**
- * This list contains 0 or more {...@link NameChangeWarning} objects.
It is
- * populated as statements are added to the
- * <code>ddlStatements</code> list. If non-empty, this list of
- * warnings should be presented to the user before the generated
- * DDL is saved or executed (to give them a chance to fix the
- * warnings and try again).
- */
- protected List<DDLWarning> warnings;
-
/**
* The name of the catalog in the target database that the
* generated DDL statements should create the objects in. Not all
@@ -157,7 +146,6 @@
public GenericDDLGenerator(boolean allowConnection) throws
SQLException {
this.allowConnection = allowConnection;
- warnings = new ArrayList<DDLWarning>();
ddlStatements = new ArrayList<DDLStatement>();
ddl = new StringBuffer(500);
println("");
@@ -202,7 +190,6 @@
* @see
ca.sqlpower.architect.ddl.DDLGenerator#generateDDLStatements(Collection)
*/
public final List<DDLStatement>
generateDDLStatements(Collection<SQLTable> tables) throws SQLException,
SQLObjectException {
- warnings = new ArrayList<DDLWarning>();
ddlStatements = new ArrayList<DDLStatement>();
ddl = new StringBuffer(500);
topLevelNames = new CaseInsensitiveHashMap();
@@ -360,7 +347,6 @@
firstColumn = true;
if (r.getChildren().isEmpty()) {
- warnings.add(new RelationshipMapsNoColumnsDDLWarning(r.getPkTable(),
r.getFkTable()));
errorMsg.append("Warning: Relationship has no columns to
map:\n");
}
@@ -371,7 +357,6 @@
// checks the fk column and pk column are the same type,
// generates DDLWarning if not the same.
if (ArchitectUtils.columnTypesDiffer(c.getType(),
fkCol.getType())) {
- warnings.add(new RelationshipColumnsTypesMismatchDDLWarning(c,
fkCol));
typesMismatchMsg.append(" " + c + " -- " + fkCol +
"\n");
}
// make sure this is unique
@@ -393,18 +378,6 @@
errorMsg.append("Warning: Column types mismatch in the following
column mapping(s):\n");
errorMsg.append(typesMismatchMsg.toString());
}
-
- // sanity check for SET NULL and SET DEFAULT delete rules, whether or
not DB supports them
- for (SQLRelationship.ColumnMapping cm :
r.getChildren(ColumnMapping.class)) {
- UpdateDeleteRule deleteRule = r.getDeleteRule();
- SQLColumn fkcol = cm.getFkColumn();
- if (deleteRule == UpdateDeleteRule.SET_NULL
&& !fkcol.isDefinitelyNullable()) {
- warnings.add(new SetNullOnNonNullableColumnWarning(fkcol));
- } else if (deleteRule == UpdateDeleteRule.SET_DEFAULT &&
- (fkcol.getDefaultValue() == null ||
fkcol.getDefaultValue().length() == 0)) {
- warnings.add(new
SetDefaultOnColumnWithNoDefaultWarning(fkcol));
- }
- }
if (supportsDeleteAction(r)) {
String deleteActionClause = getDeleteActionClause(r);
@@ -414,23 +387,9 @@
sql.append("\n").append(deleteActionClause);
}
} else {
- warnings.add(new UnsupportedFeatureDDLWarning(
- getName() + " does not support " + r.getName() + "'s delete
action", r));
errorMsg.append("Warning: " + getName() + " does not support this
relationship's " +
"delete action (" + r.getDeleteRule() + ").\n");
}
-
- // sanity check for SET NULL and SET DEFAULT update rules, whether or
not DB supports them
- for (SQLRelationship.ColumnMapping cm :
r.getChildren(SQLRelationship.ColumnMapping.class)) {
- UpdateDeleteRule updateRule = r.getUpdateRule();
- SQLColumn fkcol = cm.getFkColumn();
- if (updateRule == UpdateDeleteRule.SET_NULL
&& !fkcol.isDefinitelyNullable()) {
- warnings.add(new SetNullOnNonNullableColumnWarning(fkcol));
- } else if (updateRule == UpdateDeleteRule.SET_DEFAULT &&
- (fkcol.getDefaultValue() == null ||
fkcol.getDefaultValue().length() == 0)) {
- warnings.add(new
SetDefaultOnColumnWithNoDefaultWarning(fkcol));
- }
- }
if (supportsUpdateAction(r)) {
String updateActionClause = getUpdateActionClause(r);
@@ -440,8 +399,6 @@
sql.append("\n").append(updateActionClause);
}
} else {
- warnings.add(new UnsupportedFeatureDDLWarning(
- getName() + " does not support " + r.getName() + "'s
update action", r));
errorMsg.append("Warning: " + getName() + " does not support
this relationship's " +
"update action (" + r.getUpdateRule() + ").\n");
}
@@ -456,8 +413,6 @@
sql.append("\n").append(deferrabilityClause);
}
} else {
- warnings.add(new UnsupportedFeatureDDLWarning(
- getName() + " does not support " + r.getName() + "'s
deferrability policy", r));
errorMsg.append("Warning: " + getName() + " does not support this
relationship's " +
"deferrability policy (" + r.getDeferrability() +
").\n");
}
@@ -817,34 +772,9 @@
if (td == null) {
throw new NullPointerException("Current type map does not have
entry for default datatype!");
}
- GenericTypeDescriptor oldType = new GenericTypeDescriptor
- (c.getSourceDataTypeName(), c.getType(), c.getPrecision(),
- null, null, c.getNullable(), false, false);
- oldType.determineScaleAndPrecision();
- TypeMapDDLWarning o = new TypeMapDDLWarning(c,
String.format(
- "Type '%s' of column '%s' in table '%s' is unknown in
the target platform",
- SQLType.getTypeName(c.getType()),
- c.getPhysicalName(),
- c.getParent().getPhysicalName()), oldType, td);
- if (!contains(warnings, o)) {
- warnings.add(o);
- }
}
return td;
}
-
- private boolean contains(List<DDLWarning> list, TypeMapDDLWarning o) {
- Iterator<DDLWarning> iterator = list.iterator();
- while (iterator.hasNext()) {
- Object next = iterator.next();
- if (next instanceof TypeMapDDLWarning) {
- if
(((TypeMapDDLWarning)next).getMessage().equals(o.getMessage())) {
- return true;
- }
- }
- }
- return false;
- }
public void addTable(SQLTable t) throws SQLException,
SQLObjectException {
Map<String, SQLObject> colNameMap = new HashMap<String, SQLObject>();
// for detecting duplicate column names
@@ -1111,13 +1041,6 @@
public void setCon(Connection argCon) {
this.con = argCon;
}
-
- /**
- * Returns {...@link #warnings}.
- */
- public List<DDLWarning> getWarnings() {
- return warnings;
- }
/**
* See {...@link #targetCatalog}.
@@ -1183,157 +1106,14 @@
(so.getPhysicalName() != null
&& !so.getPhysicalName().trim().equals(""))) {
String physicalName = so.getPhysicalName();
logger.debug("The physical name for this SQLObject is: " +
physicalName);
- if (physicalName.lastIndexOf(' ') != -1) {
- String renameTo = (toIdentifier(physicalName));
- warnings.add(new InvalidNameDDLWarning(
- String.format("Name %s has white spaces",
physicalName),
- Arrays.asList(new SQLObject[] {so}),
- String.format("Rename %s to %s", physicalName, renameTo
),
- so, renameTo));
- }
} else {
so.setPhysicalName(toIdentifier(so.getName()));
}
String physicalName = so.getPhysicalName();
- if(isReservedWord(physicalName)) {
- String renameTo = physicalName + "_1";
- warnings.add(new InvalidNameDDLWarning(
- String.format("%s is a reserved word", physicalName),
- Arrays.asList(new SQLObject[] { so }),
- String.format("Rename %s to %s", physicalName,
renameTo),
- so, renameTo));
- return physicalName;
- }
logger.debug("The logical name field now is: " + so.getName());
- logger.debug("The physical name field now is: " + physicalName);
- int pointIndex = so.getPhysicalName().lastIndexOf('.');
- if
(!so.getPhysicalName().substring(pointIndex+1,pointIndex+2).matches("[a-zA-Z]")){
- String renameTo;
- if (so instanceof SQLTable) {
- renameTo = "Table_" + so.getPhysicalName();
- } else if (so instanceof SQLColumn) {
- renameTo = "Column_" + so.getPhysicalName();
- } else if (so instanceof SQLIndex) {
- renameTo = "Index_" + so.getPhysicalName();
- } else {
- renameTo = "X_" + so.getPhysicalName();
- }
- warnings.add(new InvalidNameDDLWarning(
- String.format("Name %s starts with a non-alpha
character", physicalName),
- Arrays.asList(new SQLObject[] { so }),
- String.format("Rename %s to %s", physicalName,
renameTo),
- so, renameTo));
- return physicalName;
- }
-
- logger.debug("transform identifier result: " +
so.getPhysicalName());
- // XXX should change checkDupName(Map where, so.getPhysicalName(),
so, "Duplicate Physical Name", so.getName());
-
- String physicalName2 = so.getPhysicalName();
- SQLObject object = dupCheck.get(physicalName2);
- if (object == null) {
- dupCheck.put(physicalName2, so);
- } else {
-
- int count = 1;
- String renameTo2;
- SQLObject object2;
- do {
- renameTo2 = physicalName2 + "_" + count;
- object2 = dupCheck.get(renameTo2);
- count++;
- } while (object2 != null);
-
- String message;
- if (so instanceof SQLColumn) {
- message = String.format("Column name %s in table %s
already in use",
- so.getPhysicalName(),
- ((SQLColumn) so).getParent().getPhysicalName());
- } else {
- message = String.format("Global name %s already in use",
physicalName2);
- }
- logger.debug("Rename to : " + renameTo2);
-
- warnings.add(new InvalidNameDDLWarning(
- message,
- Arrays.asList(new SQLObject[] { so }),
- String.format("Rename %s to %s", physicalName2,
renameTo2),
- so, renameTo2));
-
- dupCheck.put(renameTo2, so);
-
- }
return so.getPhysicalName();
}
-
- /**
- * Generate, set, and return a valid identifier for this SQLSequence.
- * Has a side effect of changing the given SQLColumn's
autoIncrementSequenceName.
- * @throws SQLObjectException
- *
- * @param dupCheck The Map to check for duplicate names
- * @param seq The SQLSequence to generate, set and return a
valid identifier for.
- * @param col The SQLColumn to where the side effect should
occur.
- */
- protected String createSeqPhysicalName(Map<String, SQLObject>
dupCheck, SQLSequence seq, SQLColumn col) {
- logger.debug("transform identifier source: " +
seq.getPhysicalName());
- seq.setPhysicalName(toIdentifier(seq.getName()));
- String physicalName = seq.getPhysicalName();
- if(isReservedWord(physicalName)) {
- String renameTo = physicalName + "_1";
- warnings.add(new InvalidSeqNameDDLWarning(
- String.format("%s is a reserved word", physicalName),
- seq, col,
- String.format("Rename %s to %s", physicalName,
renameTo),
- renameTo));
- return physicalName;
- }
-
- int pointIndex = seq.getPhysicalName().lastIndexOf('.');
- if
(!seq.getName().substring(pointIndex+1,pointIndex+2).matches("[a-zA-Z]")){
- String renameTo = "Seq_" + seq.getName();
- warnings.add(new InvalidSeqNameDDLWarning(
- String.format("Name %s starts with a non-alpha
character", physicalName),
- seq, col,
- String.format("Rename %s to %s", physicalName,
renameTo),
- renameTo));
- return physicalName;
- }
-
- logger.debug("transform identifier result: " +
seq.getPhysicalName());
- // XXX should change checkDupName(Map where, so.getPhysicalName(),
so, "Duplicate Physical Name", so.getName());
-
- String physicalName2 = seq.getPhysicalName();
- SQLObject object = dupCheck.get(physicalName2);
- if (object == null) {
- dupCheck.put(physicalName2, seq);
- } else {
-
- int count = 1;
- String renameTo2;
- SQLObject object2;
- do {
- renameTo2 = physicalName2 + "_" + count;
- object2 = dupCheck.get(renameTo2);
- count++;
- } while (object2 != null);
-
- String message = String.format("Global name %s already in
use", physicalName);
- logger.debug("Rename to : " + renameTo2);
-
- warnings.add(new InvalidSeqNameDDLWarning(
- message,
- seq, col,
- String.format("Rename %s to %s", physicalName,
renameTo2),
- renameTo2));
-
- dupCheck.put(renameTo2, seq);
-
- }
-
- return seq.getPhysicalName();
- }
/**
* Generates a standard <code>DROP TABLE $tablename</code> command.
Should work on most platforms.
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/LiquibaseDDLGenerator.java
Sun Jun 20 10:57:59 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/LiquibaseDDLGenerator.java
Wed Jun 30 12:59:24 2010
@@ -281,7 +281,6 @@
firstColumn = true;
if (r.getChildren().isEmpty()) {
- warnings.add(new RelationshipMapsNoColumnsDDLWarning(r.getPkTable(),
r.getFkTable()));
errorMsg.append("Warning: Relationship has no columns to
map:\n");
}
@@ -292,7 +291,6 @@
// checks the fk column and pk column are the same type,
// generates DDLWarning if not the same.
if (ArchitectUtils.columnTypesDiffer(c.getType(),
fkCol.getType())) {
- warnings.add(new RelationshipColumnsTypesMismatchDDLWarning(c,
fkCol));
typesMismatchMsg.append(" " + c + " -- " + fkCol +
"\n");
}
// make sure this is unique
=======================================
--- /trunk/src/main/java/ca/sqlpower/architect/ddl/OracleDDLGenerator.java
Fri Jun 25 11:31:05 2010
+++ /trunk/src/main/java/ca/sqlpower/architect/ddl/OracleDDLGenerator.java
Wed Jun 30 12:59:24 2010
@@ -408,7 +408,7 @@
if (c.isAutoIncrement()) {
SQLSequence seq = new
SQLSequence(toIdentifier(c.getAutoIncrementSequenceName()));
print("\nCREATE SEQUENCE ");
- print(createSeqPhysicalName(topLevelNames, seq, c));
+ print(seq.getName());
endStatement(StatementType.CREATE, seq);
}
}
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/PostgresDDLGenerator.java
Fri Jun 25 11:31:05 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/PostgresDDLGenerator.java
Wed Jun 30 12:59:24 2010
@@ -392,7 +392,7 @@
if (c.isAutoIncrement()) {
SQLSequence seq = new
SQLSequence(toIdentifier(c.getAutoIncrementSequenceName()));
print("\nCREATE SEQUENCE ");
- print(toQualifiedName(createSeqPhysicalName(topLevelNames,
seq, c)));
+ print(toQualifiedName(seq.getName()));
endStatement(StatementType.CREATE, seq);
}
}
=======================================
--- /trunk/src/main/java/ca/sqlpower/architect/ddl/critic/Critic.java Mon
Jun 14 14:02:56 2010
+++ /trunk/src/main/java/ca/sqlpower/architect/ddl/critic/Critic.java Wed
Jun 30 12:59:24 2010
@@ -36,6 +36,15 @@
* Classes of this type must be immutable.
*/
public interface Critic {
+
+ /**
+ * Start must be called once before calling criticize on this critic.
+ * Calling start on a critic allows the critic to initialize any state
for
+ * the critic for the duration of this run. If the criticism of the
state of
+ * the system is to be executed again end must be called before start
is
+ * called again.
+ */
+ public void start();
/**
* Analyzes the subject and returns a set of criticisms if there are
@@ -44,6 +53,13 @@
*/
public List<Criticism> criticize(Object subject);
+ /**
+ * End must be called when criticizing the current set of objects is
+ * complete. Calling end allows the critic to clean up any state
before it
+ * is executed again.
+ */
+ public void end();
+
/**
* The error level this critic was defined to be at when it was
created.
* The severity should not change even if settings in the project
change
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/CriticAndSettings.java
Tue Jun 29 13:26:19 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/CriticAndSettings.java
Wed Jun 30 12:59:24 2010
@@ -86,6 +86,12 @@
*/
private final String platformType;
+ /**
+ * If true the critic has been started and is not allowed to be started
+ * again until it has been ended.
+ */
+ private boolean started;
+
/**
* @param platformType
* A string that will group critics together. This is
normally a
@@ -103,6 +109,24 @@
severity = Severity.ERROR;
setName(name);
}
+
+ /**
+ * Most critics need to do nothing in terms of state. Only select
critics
+ * should ever need to override this method.
+ */
+ public void start() {
+ if (started) throw new IllegalStateException("The critic " +
getName() +
+ " has been started already.");
+ started = true;
+ }
+
+ /**
+ * Most critics need to do nothing in terms of state. Only select
critics
+ * should ever need to override this method.
+ */
+ public void end() {
+ started = false;
+ }
@Mutator
public void setSeverity(Severity severity) {
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/CriticManager.java
Tue Jun 29 13:26:19 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/CriticManager.java
Wed Jun 30 12:59:24 2010
@@ -30,6 +30,7 @@
import ca.sqlpower.architect.ddl.critic.impl.AlphaNumericNameCritic;
import
ca.sqlpower.architect.ddl.critic.impl.AlphaNumericSequenceNameCritic;
import ca.sqlpower.architect.ddl.critic.impl.DB2UnsupportedFeaturesCritic;
+import ca.sqlpower.architect.ddl.critic.impl.DuplicateNameCritic;
import ca.sqlpower.architect.ddl.critic.impl.EmptyRelationshipCritic;
import ca.sqlpower.architect.ddl.critic.impl.H2UnsupportedFeaturesCritic;
import
ca.sqlpower.architect.ddl.critic.impl.HSQLDBUnsupportedFeaturesCritic;
@@ -82,6 +83,7 @@
new AlphaNumericSequenceNameCritic(),
new SetDefaultOnColumnWithNoDefaultCritic(),
new SetNullOnNonNullableColumnCritic(),
+ new DuplicateNameCritic(),
//DB2
new DB2UnsupportedFeaturesCritic(),
//H2
=======================================
--- /trunk/src/main/java/ca/sqlpower/architect/ddl/critic/Criticizer.java
Mon Jun 14 08:38:33 2010
+++ /trunk/src/main/java/ca/sqlpower/architect/ddl/critic/Criticizer.java
Wed Jun 30 12:59:24 2010
@@ -52,7 +52,16 @@
* descendants if it is an {...@link SPObject}.
*/
public List<Criticism> criticize(Object subject) {
- return recursivelyCriticize(subject);
+ try {
+ for (Critic c : critics) {
+ c.start();
+ }
+ return recursivelyCriticize(subject);
+ } finally {
+ for (Critic c : critics) {
+ c.end();
+ }
+ }
}
/**
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/AlphaNumericNameCritic.java
Wed Jun 30 10:01:27 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/AlphaNumericNameCritic.java
Wed Jun 30 12:59:24 2010
@@ -34,17 +34,4 @@
setName(Messages.getString("AlphaNumericNameCritic.name"));
}
- @Override
- public String correctPhysicalName(String existingName) {
- StringBuffer buffer = new StringBuffer(existingName.length());
- for (int i = 0; i < existingName.length(); i++) {
- if (existingName.charAt(i) == ' ') {
- buffer.append('_');
- } else if
(getLegalNamePattern().matcher(Character.toString(existingName.charAt(i))).matches())
{
- buffer.append(existingName.charAt(i));
- }
- }
- return buffer.toString();
- }
-
-}
+}
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/OraclePhysicalNameCritic.java
Wed Jun 30 10:01:27 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/OraclePhysicalNameCritic.java
Wed Jun 30 12:59:24 2010
@@ -33,18 +33,5 @@
Pattern.compile("^[a-z_][a-z0-9_]*$",
Pattern.CASE_INSENSITIVE),
30);
}
-
- @Override
- public String correctPhysicalName(String existingName) {
- StringBuffer buffer = new StringBuffer(existingName.length());
- for (int i = 0; i < existingName.length(); i++) {
- if (existingName.charAt(i) == ' ') {
- buffer.append('_');
- } else if
(getLegalNamePattern().matcher(Character.toString(existingName.charAt(i))).matches())
{
- buffer.append(existingName.charAt(i));
- }
- }
- return buffer.toString();
- }
}
=======================================
---
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/PhysicalNameCritic.java
Wed Jun 30 10:01:27 2010
+++
/trunk/src/main/java/ca/sqlpower/architect/ddl/critic/impl/PhysicalNameCritic.java
Wed Jun 30 12:59:24 2010
@@ -31,7 +31,10 @@
import ca.sqlpower.object.annotation.Accessor;
import ca.sqlpower.object.annotation.Constructor;
import ca.sqlpower.object.annotation.ConstructorParameter;
+import ca.sqlpower.sqlobject.SQLColumn;
+import ca.sqlpower.sqlobject.SQLIndex;
import ca.sqlpower.sqlobject.SQLObject;
+import ca.sqlpower.sqlobject.SQLTable;
import ca.sqlpower.sqlobject.SQLIndex.Column;
import ca.sqlpower.sqlobject.SQLRelationship.ColumnMapping;
@@ -39,7 +42,7 @@
* Criticizes the physical name of all SQLObjects based on the parameters
given
* to the constructor.
*/
-public abstract class PhysicalNameCritic extends CriticAndSettings {
+public class PhysicalNameCritic extends CriticAndSettings {
private final Pattern legalNamePattern;
private final int maxNameLength;
@@ -111,7 +114,7 @@
}
if (!getLegalNamePattern().matcher(physName).matches()) {
- final String newLogicalName = correctPhysicalName(physName);
+ final String newLogicalName = correctPhysicalName(so,
physName);
criticisms.add(new Criticism(
so,
"Physical name not legal for " + so.getPhysicalName(),
@@ -129,12 +132,32 @@
}
/**
- * This method will be given the existing physical name of an object
being
- * criticized that does not match the pattern for valid physical names
and
- * it will return a valid physical name for the object that passes the
legal
+ * This method will be given the subject object being criticized and
its
+ * physical name that does not match the pattern for valid physical
names and it
+ * will return a valid physical name for the object that passes the
legal
* name pattern.
*/
- public abstract String correctPhysicalName(String existingName);
+ public String correctPhysicalName(Object subject, String existingName)
{
+ StringBuffer buffer = new StringBuffer(existingName.length());
+ for (int i = 0; i < existingName.length(); i++) {
+ if (existingName.charAt(i) == ' ') {
+ buffer.append('_');
+ } else if
(getLegalNamePattern().matcher(Character.toString(existingName.charAt(i))).matches())
{
+ buffer.append(existingName.charAt(i));
+ } else if (i == 0) {
+ if (subject instanceof SQLTable) {
+ buffer.append("Table_" + existingName.charAt(i));
+ } else if (subject instanceof SQLColumn) {
+ buffer.append("Column_" + existingName.charAt(i));
+ } else if (subject instanceof SQLIndex) {
+ buffer.append("Index_" + existingName.charAt(i));
+ } else {
+ buffer.append("X_" + existingName.charAt(i));
+ }
+ }
+ }
+ return buffer.toString();
+ }
@Accessor
public Pattern getLegalNamePattern() {
