Code refactoring

This commit refactors vast parts of the sqlitetypes.h interface. Its
main goals are: less code, easier code, a more modern interface, reduced
likelihood for strange errors and more flexibility for future
extensions.

The main reason why the sqlitetypes.h functions were working so well in
DB4S was not because they were that stable but because they were
extremely interlinked with the rest of the code. This is fine because we
do not plan to ship them as a separate library. But it makes it hard to
find the obvious spot to fix an issue or to put a new function. It can
always be done in the sqlitetypes function or in the rest of the DB4S
code because it is just not clear what the interface between the two
should look like. This is supposed to be improved by this commit. One
main thing here is to make ownership of objects a bit clearer.

In theory the new code should be faster too but that difference will be
neglectable from a user POV.

This commit also fixes a hidden bug which caused all table constraints
to be removed in the Edit Table dialog when a single field was removed
from the table.

This is all still WIP and more work is needed to be done here.
This commit is contained in:
Martin Kleusberg
2018-09-04 19:46:39 +02:00
parent f3e6aec57d
commit bf505edf66
16 changed files with 685 additions and 792 deletions

View File

@@ -10,6 +10,7 @@
#include <QComboBox>
#include <QDateTime>
#include <QKeyEvent>
#include <algorithm>
EditTableDialog::EditTableDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& tableName, bool createTable, QWidget* parent)
: QDialog(parent),
@@ -33,7 +34,7 @@ EditTableDialog::EditTableDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier&
if(m_bNewTable == false)
{
// Existing table, so load and set the current layout
m_table = *(pdb.getObjectByName(curTable).dynamicCast<sqlb::Table>());
m_table = *(pdb.getObjectByName<sqlb::Table>(curTable));
ui->labelEditWarning->setVisible(!m_table.fullyParsed());
// Set without rowid checkbox and schema dropdown. No need to trigger any events here as we're only loading a table exactly as it is stored by SQLite, so no need
@@ -103,22 +104,22 @@ void EditTableDialog::populateFields()
this,SLOT(itemChanged(QTreeWidgetItem*,int)));
ui->treeWidget->clear();
sqlb::FieldVector fields = m_table.fields();
sqlb::FieldVector pk = m_table.primaryKey();
for(const sqlb::FieldPtr& f : fields)
const auto& fields = m_table.fields;
QStringList pk = m_table.primaryKey();
for(const sqlb::Field& f : fields)
{
QTreeWidgetItem *tbitem = new QTreeWidgetItem(ui->treeWidget);
tbitem->setFlags(tbitem->flags() | Qt::ItemIsEditable);
tbitem->setText(kName, f->name());
tbitem->setText(kName, f.name());
QComboBox* typeBox = new QComboBox(ui->treeWidget);
typeBox->setProperty("column", f->name());
typeBox->setProperty("column", f.name());
typeBox->setEditable(true);
typeBox->addItems(sqlb::Field::Datatypes);
int index = typeBox->findText(f->type(), Qt::MatchExactly);
typeBox->addItems(DBBrowserDB::Datatypes);
int index = typeBox->findText(f.type(), Qt::MatchExactly);
if(index == -1)
{
// non standard named type
typeBox->addItem(f->type());
typeBox->addItem(f.type());
index = typeBox->count() - 1;
}
typeBox->setCurrentIndex(index);
@@ -126,22 +127,22 @@ void EditTableDialog::populateFields()
connect(typeBox, SIGNAL(currentIndexChanged(int)), this, SLOT(updateTypes()));
ui->treeWidget->setItemWidget(tbitem, kType, typeBox);
tbitem->setCheckState(kNotNull, f->notnull() ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kPrimaryKey, pk.contains(f) ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kAutoIncrement, f->autoIncrement() ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kUnique, f->unique() ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kNotNull, f.notnull() ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kPrimaryKey, contains(pk, f.name()) ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kAutoIncrement, f.autoIncrement() ? Qt::Checked : Qt::Unchecked);
tbitem->setCheckState(kUnique, f.unique() ? Qt::Checked : Qt::Unchecked);
// For the default value check if it is surrounded by parentheses and if that's the case
// add a '=' character before the entire string to match the input format we're expecting
// from the user when using functions in the default value field.
if(f->defaultValue().startsWith('(') && f->defaultValue().endsWith(')'))
tbitem->setText(kDefault, "=" + f->defaultValue());
if(f.defaultValue().startsWith('(') && f.defaultValue().endsWith(')'))
tbitem->setText(kDefault, "=" + f.defaultValue());
else
tbitem->setText(kDefault, f->defaultValue());
tbitem->setText(kDefault, f.defaultValue());
tbitem->setText(kCheck, f->check());
tbitem->setText(kCheck, f.check());
QSharedPointer<sqlb::ForeignKeyClause> fk = m_table.constraint({f}, sqlb::Constraint::ForeignKeyConstraintType).dynamicCast<sqlb::ForeignKeyClause>();
auto fk = std::dynamic_pointer_cast<sqlb::ForeignKeyClause>(m_table.constraint({f.name()}, sqlb::Constraint::ForeignKeyConstraintType));
if(fk)
tbitem->setText(kForeignKey, fk->toString());
ui->treeWidget->addTopLevelItem(tbitem);
@@ -210,14 +211,14 @@ void EditTableDialog::checkInput()
m_fkEditorDelegate->updateTablesList(oldTableName);
// update fk's that refer to table itself recursively
sqlb::FieldVector fields = m_table.fields();
for(const sqlb::FieldPtr& f : fields) {
QSharedPointer<sqlb::ForeignKeyClause> fk = m_table.constraint({f}, sqlb::Constraint::ForeignKeyConstraintType).dynamicCast<sqlb::ForeignKeyClause>();
if(!fk.isNull()) {
const auto& fields = m_table.fields;
for(const sqlb::Field& f : fields) {
auto fk = std::dynamic_pointer_cast<sqlb::ForeignKeyClause>(m_table.constraint({f.name()}, sqlb::Constraint::ForeignKeyConstraintType));
if(fk) {
if (oldTableName == fk->table()) {
fk->setTable(normTableName);
if(!m_bForeignKeysEnabled)
pdb.alterTable(curTable, m_table, f->name(), f, 0);
pdb.alterTable(curTable, m_table, f.name(), std::make_shared<sqlb::Field>(f), 0);
}
}
}
@@ -237,16 +238,16 @@ void EditTableDialog::updateTypes(QObject *object)
QString type = typeBox->currentText();
QString column = typeBox->property("column").toString();
int index;
for(index=0; index < m_table.fields().size(); ++index)
size_t index;
for(index=0; index < m_table.fields.size(); ++index)
{
if(m_table.fields().at(index)->name() == column)
if(m_table.fields.at(index).name() == column)
break;
}
m_table.fields().at(index)->setType(type);
m_table.fields.at(index).setType(type);
if(!m_bNewTable)
pdb.alterTable(curTable, m_table, column, m_table.fields().at(index));
pdb.alterTable(curTable, m_table, column, std::make_shared<sqlb::Field>(m_table.fields.at(index)));
checkInput();
}
}
@@ -268,11 +269,11 @@ bool EditTableDialog::eventFilter(QObject *object, QEvent *event)
void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
{
int index = ui->treeWidget->indexOfTopLevelItem(item);
if(index < m_table.fields().count())
if(index < static_cast<int>(m_table.fields.size()))
{
sqlb::FieldPtr field = m_table.fields().at(index);
sqlb::Field& field = m_table.fields.at(index);
bool callRenameColumn = false;
QString oldFieldName = field->name();
QString oldFieldName = field.name();
switch(column)
{
@@ -284,8 +285,8 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
// with different case. Example: if I rename column 'COLUMN' to 'column', findField() is going to return the current field number
// because it's doing a case-independent search and it can't return another field number because SQLite prohibits duplicate field
// names (no matter the case). So when this happens we just allow the renaming because there's no harm to be expected from it.
int foundField = m_table.findField(item->text(column));
if(foundField != -1 && foundField != index)
auto foundField = sqlb::findField(m_table, item->text(column));
if(foundField != m_table.fields.end() && foundField-m_table.fields.begin() != index)
{
QMessageBox::warning(this, qApp->applicationName(), tr("There already is a field with that name. Please rename it first or choose a different "
"name for this field."));
@@ -299,16 +300,16 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
// When editing an exiting table, check if any foreign keys would cause trouble in case this name is edited
if(!m_bNewTable)
{
sqlb::FieldVector pk = m_table.primaryKey();
QStringList pk = m_table.primaryKey();
for(const sqlb::ObjectPtr& fkobj : pdb.schemata[curTable.schema()].values("table"))
{
QList<sqlb::ConstraintPtr> fks = fkobj.dynamicCast<sqlb::Table>()->constraints(sqlb::FieldVector(), sqlb::Constraint::ForeignKeyConstraintType);
auto fks = std::dynamic_pointer_cast<sqlb::Table>(fkobj)->constraints(QStringList(), sqlb::Constraint::ForeignKeyConstraintType);
for(const sqlb::ConstraintPtr& fkptr : fks)
{
QSharedPointer<sqlb::ForeignKeyClause> fk = fkptr.dynamicCast<sqlb::ForeignKeyClause>();
auto fk = std::dynamic_pointer_cast<sqlb::ForeignKeyClause>(fkptr);
if(fk->table() == m_table.name())
{
if(fk->columns().contains(field->name()) || pk.contains(field))
if(fk->columns().contains(field.name()) || contains(pk, field.name()))
{
QMessageBox::warning(this, qApp->applicationName(), tr("This column is referenced in a foreign key in table %1 and thus "
"its name cannot be changed.")
@@ -324,7 +325,8 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
}
}
field->setName(item->text(column));
field.setName(item->text(column));
m_table.renameKeyInAllConstraints(oldFieldName, item->text(column));
qobject_cast<QComboBox*>(ui->treeWidget->itemWidget(item, kType))->setProperty("column", item->text(column));
if(!m_bNewTable)
callRenameColumn = true;
@@ -335,28 +337,17 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
case kPrimaryKey:
{
// Check if there already is a primary key
if(m_table.constraint(sqlb::FieldVector(), sqlb::Constraint::PrimaryKeyConstraintType))
if(m_table.constraint(QStringList(), sqlb::Constraint::PrimaryKeyConstraintType))
{
// There already is a primary key for this table. So edit that one as there always can only be one primary key anyway.
sqlb::FieldVector& pk = m_table.primaryKeyRef();
QStringList& pk = m_table.primaryKeyRef();
if(item->checkState(column) == Qt::Checked)
pk.push_back(field);
pk.push_back(field.name());
else
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
pk.removeAll(field);
#else
{
int idx = pk.indexOf (field);
while ( idx != -1 )
{
pk.remove (idx);
idx = pk.indexOf (field);
}
}
#endif
pk.erase(std::remove(pk.begin(), pk.end(), field.name()), pk.end());
} else if(item->checkState(column) == Qt::Checked) {
// There is no primary key in the table yet. This means we need to add a default one.
m_table.addConstraint({field}, sqlb::ConstraintPtr(new sqlb::PrimaryKeyConstraint()));
m_table.addConstraint({field.name()}, sqlb::ConstraintPtr(new sqlb::PrimaryKeyConstraint()));
}
if(item->checkState(column) == Qt::Checked)
@@ -385,9 +376,9 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
// to at least replace all troublesome NULL values by the default value
SqliteTableModel m(pdb, this);
m.setQuery(QString("SELECT COUNT(%1) FROM %2 WHERE %3 IS NULL;")
.arg(sqlb::escapeIdentifier(pdb.getObjectByName(curTable).dynamicCast<sqlb::Table>()->rowidColumn()))
.arg(sqlb::escapeIdentifier(pdb.getObjectByName<sqlb::Table>(curTable)->rowidColumn()))
.arg(curTable.toString())
.arg(sqlb::escapeIdentifier(field->name())));
.arg(sqlb::escapeIdentifier(field.name())));
if(!m.completeCache())
{
// If we couldn't load all data because the cancel button was clicked, just unset the checkbox again and stop.
@@ -403,7 +394,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
return;
}
}
field->setNotNull(item->checkState(column) == Qt::Checked);
field.setNotNull(item->checkState(column) == Qt::Checked);
if(!m_bNewTable)
callRenameColumn = true;
}
@@ -419,8 +410,8 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
SqliteTableModel m(pdb, this);
m.setQuery(QString("SELECT COUNT(*) FROM %1 WHERE %2 <> CAST(%3 AS INTEGER);")
.arg(curTable.toString())
.arg(sqlb::escapeIdentifier(field->name()))
.arg(sqlb::escapeIdentifier(field->name())));
.arg(sqlb::escapeIdentifier(field.name()))
.arg(sqlb::escapeIdentifier(field.name())));
if(!m.completeCache())
{
// If we couldn't load all data because the cancel button was clicked, just unset the checkbox again and stop.
@@ -454,7 +445,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
}
}
}
field->setAutoIncrement(ischecked);
field.setAutoIncrement(ischecked);
if(!m_bNewTable)
callRenameColumn = true;
@@ -467,7 +458,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
{
// Because our renameColumn() function fails when setting a column to unique when it already contains the same values
SqliteTableModel m(pdb, this);
m.setQuery(QString("SELECT COUNT(%2) FROM %1;").arg(curTable.toString()).arg(sqlb::escapeIdentifier(field->name())));
m.setQuery(QString("SELECT COUNT(%2) FROM %1;").arg(curTable.toString()).arg(sqlb::escapeIdentifier(field.name())));
if(!m.completeCache())
{
// If we couldn't load all data because the cancel button was clicked, just unset the checkbox again and stop.
@@ -475,7 +466,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
return;
}
int rowcount = m.data(m.index(0, 0)).toInt();
m.setQuery(QString("SELECT COUNT(DISTINCT %2) FROM %1;").arg(curTable.toString()).arg(sqlb::escapeIdentifier(field->name())));
m.setQuery(QString("SELECT COUNT(DISTINCT %2) FROM %1;").arg(curTable.toString()).arg(sqlb::escapeIdentifier(field.name())));
if(!m.completeCache())
{
// If we couldn't load all data because the cancel button was clicked, just unset the checkbox again and stop.
@@ -486,13 +477,13 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
if(rowcount != uniquecount)
{
// There is a NULL value, so print an error message, uncheck the combobox, and return here
QMessageBox::information(this, qApp->applicationName(), tr("Column '%1' has no unique data.\n").arg(field->name())
QMessageBox::information(this, qApp->applicationName(), tr("Column '%1' has no unique data.\n").arg(field.name())
+ tr("This makes it impossible to set this flag. Please change the table data first."));
item->setCheckState(column, Qt::Unchecked);
return;
}
}
field->setUnique(item->checkState(column) == Qt::Checked);
field.setUnique(item->checkState(column) == Qt::Checked);
if(!m_bNewTable)
callRenameColumn = true;
@@ -525,13 +516,13 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
}
}
}
field->setDefaultValue(new_value);
field.setDefaultValue(new_value);
if(!m_bNewTable)
callRenameColumn = true;
}
break;
case kCheck:
field->setCheck(item->text(column));
field.setCheck(item->text(column));
if(!m_bNewTable)
callRenameColumn = true;
break;
@@ -545,7 +536,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column)
if(callRenameColumn)
{
if(!pdb.alterTable(curTable, m_table, oldFieldName, field))
if(!pdb.alterTable(curTable, m_table, oldFieldName, std::make_shared<sqlb::Field>(field)))
QMessageBox::warning(this, qApp->applicationName(), tr("Modifying this column failed. Error returned from database:\n%1").arg(pdb.lastError()));
}
}
@@ -567,18 +558,18 @@ void EditTableDialog::addField()
{
field_name = "Field" + QString::number(field_number);
field_number++;
} while(m_table.findField(field_name) != -1);
} while(sqlb::findField(m_table, field_name) != m_table.fields.end());
tbitem->setText(kName, field_name);
}
QComboBox* typeBox = new QComboBox(ui->treeWidget);
typeBox->setProperty("column", tbitem->text(kName));
typeBox->setEditable(true);
typeBox->addItems(sqlb::Field::Datatypes);
typeBox->addItems(DBBrowserDB::Datatypes);
int defaultFieldTypeIndex = Settings::getValue("db", "defaultfieldtype").toInt();
if (defaultFieldTypeIndex < sqlb::Field::Datatypes.count())
if (defaultFieldTypeIndex < DBBrowserDB::Datatypes.count())
{
typeBox->setCurrentIndex(defaultFieldTypeIndex);
}
@@ -597,15 +588,11 @@ void EditTableDialog::addField()
ui->treeWidget->editItem(tbitem, 0);
// add field to table object
sqlb::FieldPtr f(new sqlb::Field(
tbitem->text(kName),
typeBox->currentText()
));
m_table.addField(f);
m_table.fields.emplace_back(tbitem->text(kName), typeBox->currentText());
// Actually add the new column to the table if we're editing an existing table
if(!m_bNewTable)
pdb.addColumn(curTable, f);
pdb.addColumn(curTable, m_table.fields.back());
checkInput();
}
@@ -622,9 +609,8 @@ void EditTableDialog::removeField()
// Creating a new one
// Just delete that item. At this point there is no DB table to edit or data to be lost anyway
sqlb::FieldVector fields = m_table.fields();
fields.remove(ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem()));
m_table.setFields(fields);
m_table.fields.erase(m_table.fields.begin() + ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem()));
m_table.removeKeyFromAllConstraints(ui->treeWidget->currentItem()->text(kName));
delete ui->treeWidget->currentItem();
} else {
// Editing an old one
@@ -638,7 +624,7 @@ void EditTableDialog::removeField()
QMessageBox::warning(nullptr, QApplication::applicationName(), pdb.lastError());
} else {
//relayout
m_table = *(pdb.getObjectByName(curTable).dynamicCast<sqlb::Table>());
m_table = *(pdb.getObjectByName<sqlb::Table>(curTable));
populateFields();
}
}
@@ -704,9 +690,7 @@ void EditTableDialog::moveCurrentField(bool down)
ui->treeWidget->setCurrentIndex(ui->treeWidget->currentIndex().sibling(newRow, 0));
// Finally update the table SQL
sqlb::FieldVector fields = m_table.fields();
std::swap(fields[newRow], fields[currentRow]);
m_table.setFields(fields);
std::swap(m_table.fields[newRow], m_table.fields[currentRow]);
} else {
// Editing an old one
@@ -715,14 +699,14 @@ void EditTableDialog::moveCurrentField(bool down)
curTable,
m_table,
ui->treeWidget->currentItem()->text(0),
m_table.fields().at(ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem())),
std::make_shared<sqlb::Field>(m_table.fields.at(ui->treeWidget->indexOfTopLevelItem(ui->treeWidget->currentItem()))),
(down ? 1 : -1)
))
{
QMessageBox::warning(nullptr, QApplication::applicationName(), pdb.lastError());
} else {
// Reload table SQL
m_table = *(pdb.getObjectByName(curTable).dynamicCast<sqlb::Table>());
m_table = *(pdb.getObjectByName<sqlb::Table>(curTable));
populateFields();
// Select old item at new position
@@ -739,8 +723,8 @@ void EditTableDialog::setWithoutRowid(bool without_rowid)
if(without_rowid)
{
// Before setting the without rowid flag, first perform a check to see if the table meets all the required criteria for without rowid tables
int pk = m_table.findPk();
if(pk == -1 || m_table.fields().at(pk)->autoIncrement())
auto pk = m_table.findPk();
if(pk == m_table.fields.end() || pk->autoIncrement())
{
QMessageBox::information(this, QApplication::applicationName(),
tr("Please add a field which meets the following criteria before setting the without rowid flag:\n"
@@ -756,7 +740,7 @@ void EditTableDialog::setWithoutRowid(bool without_rowid)
}
// If it does, override the the rowid column name of the table object with the name of the primary key.
m_table.setRowidColumn(m_table.fields().at(pk)->name());
m_table.setRowidColumn(pk->name());
} else {
// If the without rowid flag is unset no further checks are required. Just set the rowid column name back to "_rowid_"
m_table.setRowidColumn("_rowid_");