Dive site: remove UUIDs from LocationInformationModel

Replace UUIDs from LocationInformationModel and fix the fallout.
Notably, replace the UUID "column" by a DIVESITE "column".
Getting pointers through Qt's QVariant is horrible, we'll have
to think about a better solution.

RECENTLY_ADDED_DIVESITE now defines to a special pointer to
struct dive_site (defined as ~0).

This fixes an interesting logic bug:
The old code checked the uuid of the LocationInformationModel (currUuid)
for the value "1", which corresponded to RECENTLY_ADDED_DIVESITE.
If equal, currType would be set to NEW_DIVE_SITE. Later, _currType_
was compared against _RECENTLY_ADDED_DIVESITE_. This would only work
because NEW_DIVE_SITE and RECENTLY_ADDED_DIVESITE both were defined
as 1.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

desktop-widgets/locationinformation.cpp

@@ -225,7 +225,7 @@ void LocationInformationWidget::initFields(dive_site *ds)
 	diveSite = ds;
 	if (ds) {
 		copy_taxonomy(&ds->taxonomy, &taxonomy);
-		filter_model.set(ds->uuid, ds->location);
+		filter_model.set(ds, ds->location);
 		updateLabels();
 		enableLocationButtons(dive_site_has_gps_location(ds));
 		QSortFilterProxyModel *m = qobject_cast<QSortFilterProxyModel *>(ui.diveSiteListView->model());
@@ -382,9 +382,8 @@ QVariant DiveLocationModel::data(const QModelIndex &index, int role) const
 	static const QIcon geoCode(":geotag-icon");
 
 	if (index.row() <= 1) { // two special cases.
-		if (index.column() == LocationInformationModel::UUID) {
-			return RECENTLY_ADDED_DIVESITE;
-		}
+		if (index.column() == LocationInformationModel::DIVESITE)
+			return QVariant::fromValue<void *>(RECENTLY_ADDED_DIVESITE);
 		switch (role) {
 		case Qt::DisplayRole:
 			return new_ds_value[index.row()];
@@ -429,9 +428,9 @@ DiveLocationLineEdit::DiveLocationLineEdit(QWidget *parent) : QLineEdit(parent),
 							      proxy(new DiveLocationFilterProxyModel()),
 							      model(new DiveLocationModel()),
 							      view(new DiveLocationListView()),
-							      currType(NO_DIVE_SITE)
+							      currType(NO_DIVE_SITE),
+							      currDs(nullptr)
 {
-	currUuid = 0;
 	location_line_edit = this;
 
 	proxy->setSourceModel(model);
@@ -507,15 +506,14 @@ void DiveLocationLineEdit::focusOutEvent(QFocusEvent *ev)
 void DiveLocationLineEdit::itemActivated(const QModelIndex &index)
 {
 	QModelIndex idx = index;
-	if (index.column() == LocationInformationModel::UUID)
+	if (index.column() == LocationInformationModel::DIVESITE)
 		idx = index.model()->index(index.row(), LocationInformationModel::NAME);
 
-	QModelIndex uuidIndex = index.model()->index(index.row(), LocationInformationModel::UUID);
-	uint32_t uuid = uuidIndex.data().toInt();
-	currType = uuid == 1 ? NEW_DIVE_SITE : EXISTING_DIVE_SITE;
-	currUuid = uuid;
+	dive_site *ds = (dive_site *)index.model()->index(index.row(), LocationInformationModel::DIVESITE).data().value<void *>();
+	currType = ds == RECENTLY_ADDED_DIVESITE ? NEW_DIVE_SITE : EXISTING_DIVE_SITE;
+	currDs = ds;
 	setText(idx.data().toString());
-	if (currUuid == NEW_DIVE_SITE)
+	if (currType == NEW_DIVE_SITE)
 		qDebug() << "Setting a New dive site";
 	else
 		qDebug() << "Setting a Existing dive site";
@@ -580,7 +578,7 @@ void DiveLocationLineEdit::keyPressEvent(QKeyEvent *ev)
 
 		if (ev->key() != Qt::Key_Up && ev->key() != Qt::Key_Down) {
 			currType = NEW_DIVE_SITE;
-			currUuid = RECENTLY_ADDED_DIVESITE;
+			currDs = RECENTLY_ADDED_DIVESITE;
 		} else {
 			showPopup();
 		}
@@ -626,17 +624,15 @@ void DiveLocationLineEdit::fixPopupPosition()
 	}
 }
 
-void DiveLocationLineEdit::setCurrentDiveSiteUuid(uint32_t uuid)
+void DiveLocationLineEdit::setCurrentDiveSite(struct dive_site *ds)
 {
-	currUuid = uuid;
-	if (uuid == 0) {
+	currDs = ds;
+	if (!currDs) {
 		currType = NO_DIVE_SITE;
-	}
-	struct dive_site *ds = get_dive_site_by_uuid(uuid);
-	if (!ds)
 		clear();
-	else
+	} else {
 		setText(ds->name);
+	}
 }
 
 void DiveLocationLineEdit::showPopup()
@@ -654,9 +650,9 @@ DiveLocationLineEdit::DiveSiteType DiveLocationLineEdit::currDiveSiteType() cons
 	return currType;
 }
 
-uint32_t DiveLocationLineEdit::currDiveSiteUuid() const
+struct dive_site *DiveLocationLineEdit::currDiveSite() const
 {
-	return currUuid;
+	return currDs;
 }
 
 DiveLocationListView::DiveLocationListView(QWidget*)

desktop-widgets/locationinformation.h

@@ -96,9 +96,9 @@ public:
 	bool eventFilter(QObject*, QEvent*);
 	void itemActivated(const QModelIndex& index);
 	DiveSiteType currDiveSiteType() const;
-	uint32_t currDiveSiteUuid() const;
+	struct dive_site *currDiveSite() const;
 	void fixPopupPosition();
-	void setCurrentDiveSiteUuid(uint32_t uuid);
+	void setCurrentDiveSite(struct dive_site *ds);
 
 signals:
 	void diveSiteSelected();
@@ -116,7 +116,7 @@ private:
 	DiveLocationModel *model;
 	DiveLocationListView *view;
 	DiveSiteType currType;
-	uint32_t currUuid;
+	struct dive_site *currDs;
 };
 
 #endif

desktop-widgets/mapwidget.cpp

@@ -8,6 +8,7 @@
 #include "core/divesite.h"
 #include "map-widget/qmlmapwidgethelper.h"
 #include "qt-models/maplocationmodel.h"
+#include "qt-models/divelocationmodel.h"
 #include "mainwindow.h"
 #include "divelistview.h"
 
@@ -66,8 +67,8 @@ void MapWidget::centerOnDiveSite(struct dive_site *ds)
 void MapWidget::centerOnIndex(const QModelIndex& idx)
 {
 	CHECK_IS_READY_RETURN_VOID();
-	struct dive_site *ds = get_dive_site_by_uuid(idx.model()->index(idx.row(), 0).data().toUInt());
-	if (!ds || !dive_site_has_gps_location(ds))
+	struct dive_site *ds = (struct dive_site *)idx.model()->index(idx.row(), LocationInformationModel::DIVESITE).data().value<void *>();
+	if (!ds || ds == RECENTLY_ADDED_DIVESITE || !dive_site_has_gps_location(ds))
 		centerOnSelectedDiveSite();
 	else
 		centerOnDiveSite(ds);

desktop-widgets/modeldelegates.cpp

@@ -11,6 +11,7 @@
 #include "qt-models/weightsysteminfomodel.h"
 #include "qt-models/weightmodel.h"
 #include "qt-models/divetripmodel.h"
+#include "qt-models/divelocationmodel.h"
 #include "core/qthelper.h"
 
 #include <QCompleter>
@@ -452,9 +453,8 @@ void LocationFilterDelegate::paint(QPainter *painter, const QStyleOptionViewItem
 	QString diveSiteName = index.data().toString();
 	QString bottomText;
 	QIcon icon = index.data(Qt::DecorationRole).value<QIcon>();
-	struct dive_site *ds = get_dive_site_by_uuid(
-		index.model()->data(index.model()->index(index.row(),0)).toInt()
-	);
+	struct dive_site *ds = (struct dive_site *)
+		index.model()->data(index.model()->index(index.row(), LocationInformationModel::DIVESITE)).value<void *>();
 	struct dive_site *currentDiveSite = current_dive ? get_dive_site_for_dive(current_dive) : nullptr;
 	bool currentDiveSiteHasGPS = currentDiveSite && dive_site_has_gps_location(currentDiveSite);
 	//Special case: do not show name, but instead, show

desktop-widgets/tab-widgets/maintab.cpp

@@ -224,7 +224,7 @@ void MainTab::setCurrentLocationIndex()
 	if (current_dive) {
 		struct dive_site *ds = get_dive_site_by_uuid(current_dive->dive_site_uuid);
 		if (ds)
-			ui.location->setCurrentDiveSiteUuid(ds->uuid);
+			ui.location->setCurrentDiveSite(ds);
 		else
 			ui.location->clear();
 	}
@@ -417,7 +417,7 @@ void MainTab::updateDiveInfo(bool clear)
 		struct dive_site *ds = NULL;
 		ds = get_dive_site_by_uuid(displayed_dive.dive_site_uuid);
 		if (ds) {
-			ui.location->setCurrentDiveSiteUuid(ds->uuid);
+			ui.location->setCurrentDiveSite(ds);
 			ui.locationTags->setText(constructLocationTags(&ds->taxonomy, true));
 
 			if (ui.locationTags->text().isEmpty() && has_location(&ds->location)) {
@@ -662,12 +662,12 @@ void MainTab::refreshDisplayedDiveSite()
 {
 	struct dive_site *ds = get_dive_site_for_dive(&displayed_dive);
 	if (ds)
-		ui.location->setCurrentDiveSiteUuid(ds->uuid);
+		ui.location->setCurrentDiveSite(ds);
 }
 
 // when this is called we already have updated the current_dive and know that it exists
 // there is no point in calling this function if there is no current dive
-uint32_t MainTab::updateDiveSite(uint32_t pickedUuid, dive *d)
+struct dive_site *MainTab::updateDiveSite(struct dive_site *pickedDs, dive *d)
 {
 	if (!d)
 		return 0;
@@ -675,42 +675,40 @@ uint32_t MainTab::updateDiveSite(uint32_t pickedUuid, dive *d)
 	if (ui.location->text().isEmpty())
 		return 0;
 
-	if (pickedUuid == 0)
+	if (!pickedDs)
 		return 0;
 
 	const uint32_t origUuid = d->dive_site_uuid;
 	struct dive_site *origDs = get_dive_site_by_uuid(origUuid);
-	struct dive_site *newDs = NULL;
 	bool createdNewDive = false;
 
-	if (pickedUuid == origUuid)
-		return origUuid;
+	if (pickedDs == origDs)
+		return origDs;
 
-	if (pickedUuid == RECENTLY_ADDED_DIVESITE) {
+	if (pickedDs == RECENTLY_ADDED_DIVESITE) {
 		QString name = ui.location->text().isEmpty() ? tr("New dive site") : ui.location->text();
-		pickedUuid = create_dive_site(qPrintable(name), displayed_dive.when)->uuid;
+		pickedDs = create_dive_site(qPrintable(name), displayed_dive.when);
 		createdNewDive = true;
 		LocationFilterModel::instance()->addName(name);
 	}
 
-	newDs = get_dive_site_by_uuid(pickedUuid);
-
 	if (origDs) {
 		if(createdNewDive) {
-			copy_dive_site(origDs, newDs);
-			free(newDs->name);
-			newDs->name = copy_qstring(ui.location->text());
-			newDs->uuid = pickedUuid;
+			uint32_t pickedUuid = pickedDs->uuid;
+			copy_dive_site(origDs, pickedDs);
+			free(pickedDs->name);
+			pickedDs->name = copy_qstring(ui.location->text());
+			pickedDs->uuid = pickedUuid;
 			qDebug() << "Creating and copying dive site";
-		} else if (!has_location(&newDs->location)) {
-			newDs->location = origDs->location;
+		} else if (!has_location(&pickedDs->location)) {
+			pickedDs->location = origDs->location;
 			qDebug() << "Copying GPS information";
 		}
 	}
 
-	d->dive_site_uuid = pickedUuid;
-	qDebug() << "Setting the dive site id on the dive:" << pickedUuid;
-	return pickedUuid;
+	d->dive_site_uuid = pickedDs->uuid;
+	qDebug() << "Setting the dive site id on the dive:" << pickedDs->uuid;
+	return pickedDs;
 }
 
 // Get the list of selected dives, but put the current dive at the last position of the vector
@@ -745,7 +743,7 @@ void MainTab::acceptChanges()
 	ui.equipmentTab->setEnabled(true);
 	if (editMode == ADD) {
 		// make sure that the dive site is handled as well
-		updateDiveSite(ui.location->currDiveSiteUuid(), &displayed_dive);
+		updateDiveSite(ui.location->currDiveSite(), &displayed_dive);
 		copyTagsToDisplayedDive();
 
 		Command::addDive(&displayed_dive, autogroup, true);
@@ -880,10 +878,10 @@ void MainTab::acceptChanges()
 
 		// update the dive site for the selected dives that had the same dive site as the current dive
 		struct dive_site *oldDs = get_dive_site_by_uuid(cd->dive_site_uuid);
-		uint32_t newUuid = 0;
+		struct dive_site *newDs = 0;
 		MODIFY_DIVES(selectedDives,
 			if (mydive->dive_site_uuid == current_dive->dive_site_uuid)
-				newUuid = updateDiveSite(newUuid == 0 ? ui.location->currDiveSiteUuid() : newUuid, mydive);
+				newDs = updateDiveSite(!newDs ? ui.location->currDiveSite() : newDs, mydive);
 		);
 		if (oldDs && !is_dive_site_used(oldDs, false)) {
 			if (verbose)
@@ -1345,7 +1343,7 @@ void MainTab::on_location_diveSiteSelected()
 		emit diveSiteChanged();
 		return;
 	} else {
-		if (ui.location->currDiveSiteUuid() != displayed_dive.dive_site_uuid) {
+		if (ui.location->currDiveSite() != get_dive_site_by_uuid(displayed_dive.dive_site_uuid)) {
 			markChangedWidget(ui.location);
 		} else {
 			QPalette p;
@@ -1482,7 +1480,7 @@ void MainTab::showAndTriggerEditSelective(struct dive_components what)
 	if (what.visibility)
 		ui.visibility->setCurrentStars(displayed_dive.visibility);
 	if (what.divesite)
-		ui.location->setCurrentDiveSiteUuid(displayed_dive.dive_site_uuid);
+		ui.location->setCurrentDiveSite(get_dive_site_by_uuid(displayed_dive.dive_site_uuid));
 	if (what.tags) {
 		ui.tagWidget->setText(get_taglist_string(displayed_dive.tag_list));
 	}

desktop-widgets/tab-widgets/maintab.h

@@ -123,7 +123,7 @@ private:
 	dive_trip_t *currentTrip;
 	dive_trip_t displayedTrip;
 	bool acceptingEdit;
-	uint32_t updateDiveSite(uint32_t pickedUuid, dive *d);
+	struct dive_site *updateDiveSite(struct dive_site *pickedDs, dive *d);
 	QList<TabBase*> extraWidgets;
 };
 

qt-models/divelocationmodel.cpp

@@ -41,7 +41,7 @@ QVariant LocationInformationModel::getDiveSiteData(const struct dive_site *ds, i
 	case Qt::EditRole:
 	case Qt::DisplayRole :
 		switch(column) {
-		case UUID: return ds->uuid;
+		case DIVESITE: return QVariant::fromValue<void*>((void *)ds); // Not nice: casting away const
 		case NAME: return ds->name;
 		case LATITUDE: return ds->location.lat.udeg;
 		case LONGITUDE: return ds->location.lon.udeg;
@@ -102,7 +102,8 @@ bool LocationInformationModel::removeRows(int row, int, const QModelIndex&)
 	return true;
 }
 
-GeoReferencingOptionsModel *GeoReferencingOptionsModel::instance() {
+GeoReferencingOptionsModel *GeoReferencingOptionsModel::instance()
+{
 	static GeoReferencingOptionsModel *self = new GeoReferencingOptionsModel();
 	return self;
 }
@@ -118,24 +119,23 @@ GeoReferencingOptionsModel::GeoReferencingOptionsModel(QObject *parent) : QStrin
 
 bool GPSLocationInformationModel::filterAcceptsRow(int sourceRow, const QModelIndex &parent) const
 {
-	uint32_t uuid = sourceModel()->index(sourceRow, LocationInformationModel::UUID, parent).data().toUInt();
-	if (uuid == ignoreUuid || uuid == RECENTLY_ADDED_DIVESITE)
+	struct dive_site *ds = (struct dive_site *)sourceModel()->index(sourceRow, LocationInformationModel::DIVESITE, parent).data().value<void *>();
+	if (ds == ignoreDs || ds == RECENTLY_ADDED_DIVESITE)
 		return false;
-	struct dive_site *ds = get_dive_site_by_uuid(uuid);
 
 	return ds && same_location(&ds->location, &location);
 }
 
 GPSLocationInformationModel::GPSLocationInformationModel(QObject *parent) : QSortFilterProxyModel(parent),
-	ignoreUuid(0),
+	ignoreDs(nullptr),
 	location({{0},{0}})
 {
 	setSourceModel(LocationInformationModel::instance());
 }
 
-void GPSLocationInformationModel::set(uint32_t ignoreUuidIn, const location_t &locationIn)
+void GPSLocationInformationModel::set(const struct dive_site *ignoreDsIn, const location_t &locationIn)
 {
-	ignoreUuid = ignoreUuidIn;
+	ignoreDs = ignoreDsIn;
 	location = locationIn;
 	invalidate();
 }

qt-models/divelocationmodel.h

@@ -8,14 +8,14 @@
 #include <stdint.h>
 #include "core/units.h"
 
-#define RECENTLY_ADDED_DIVESITE 1
+#define RECENTLY_ADDED_DIVESITE ((struct dive_site *)~0)
 
 class LocationInformationModel : public QAbstractTableModel {
 Q_OBJECT
 public:
 	// Common columns, roles and accessor function for all dive-site models.
 	// Thus, different views can connect to different models.
-	enum Columns { UUID, NAME, LATITUDE, LONGITUDE, COORDS, DESCRIPTION, NOTES, TAXONOMY_1, TAXONOMY_2, TAXONOMY_3, COLUMNS};
+	enum Columns { DIVESITE, NAME, LATITUDE, LONGITUDE, COORDS, DESCRIPTION, NOTES, TAXONOMY_1, TAXONOMY_2, TAXONOMY_3, COLUMNS};
 	enum Roles { DIVESITE_ROLE = Qt::UserRole + 1 };
 	static QVariant getDiveSiteData(const struct dive_site *ds, int column, int role);
 
@@ -37,12 +37,12 @@ private:
 class GPSLocationInformationModel : public QSortFilterProxyModel {
 Q_OBJECT
 private:
-	uint32_t ignoreUuid;
+	const struct dive_site *ignoreDs;
 	location_t location;
 	bool filterAcceptsRow(int sourceRow, const QModelIndex &source_parent) const override;
 public:
 	GPSLocationInformationModel(QObject *parent = nullptr);
-	void set(uint32_t ignoreUuid, const location_t &);
+	void set(const struct dive_site *ignoreDs, const location_t &);
 	void setCoordinates(const location_t &);
 };