Make created dive site uuid deterministic

Having random uuids seemed like a good idea, but there are several
situations where they really cause problems. One is merging dive file
imports from V2 logfiles. Another is testing such imports.

Instead of making the uuid random we now hash the name and add the
timestamp of the first dive associated with this dive site to the hash
(first in this context is "first encountered" with no guarantee that it is
the chronologically first). This way V2 imports create deterministic uuids
but uuid conflicts are still extremely unlikely, even if the user has
multiple dive sites with the same name.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

11 files changed, 59 insertions, 36 deletions

diff --git a/datatrak.c b/datatrak.c
index c2764b4ef..37418c9da 100644
--- a/datatrak.c
+++ b/datatrak.c
@@ -222,7 +222,7 @@ static struct dive dt_dive_parser(FILE *archivo, struct dive *dt_dive)
 	snprintf(buffer, sizeof(buffer), "%s, %s", locality, dive_point);
 	dt_dive->dive_site_uuid = get_dive_site_uuid_by_name(buffer, NULL);
 	if (dt_dive->dive_site_uuid == 0)
-		dt_dive->dive_site_uuid = create_dive_site(buffer);
+		dt_dive->dive_site_uuid = create_dive_site(buffer, dt_dive->when);
 	free(locality);
 	free(dive_point);
 
diff --git a/dive.c b/dive.c
index 8aac690a3..4e902d7d2 100644
--- a/dive.c
+++ b/dive.c
@@ -3033,7 +3033,7 @@ void dive_set_geodata_from_picture(struct dive *dive, struct picture *picture)
 			ds->latitude = picture->latitude;
 			ds->longitude = picture->longitude;
 		} else {
-			dive->dive_site_uuid = create_dive_site_with_gps("", picture->latitude, picture->longitude);
+			dive->dive_site_uuid = create_dive_site_with_gps("", picture->latitude, picture->longitude, dive->when);
 		}
 	}
 }
diff --git a/divesite.c b/divesite.c
index 4ffdcd78f..035c1b82d 100644
--- a/divesite.c
+++ b/divesite.c
@@ -104,10 +104,13 @@ struct dive_site *alloc_dive_site(uint32_t uuid)
 		exit(1);
 	sites[nr] = ds;
 	dive_site_table.nr = nr + 1;
-	if (uuid)
+	if (uuid) {
+		if (get_dive_site_by_uuid(uuid))
+			fprintf(stderr, "PROBLEM: duplicate uuid %08x\n", uuid);
 		ds->uuid = uuid;
-	else
+	} else {
 		ds->uuid = dive_site_getUniqId();
+	}
 	return ds;
 }
 
@@ -157,19 +160,33 @@ void delete_dive_site(uint32_t id)
 	}
 }
 
+uint32_t create_divesite_uuid(const char *name, timestamp_t divetime)
+{
+	unsigned char hash[20];
+	SHA_CTX ctx;
+	SHA1_Init(&ctx);
+	SHA1_Update(&ctx, &divetime, sizeof(timestamp_t));
+	SHA1_Update(&ctx, name, strlen(name));
+	SHA1_Final(hash, &ctx);
+	// now return the first 32 of the 160 bit hash
+	return *(uint32_t *)hash;
+}
+
 /* allocate a new site and add it to the table */
-uint32_t create_dive_site(const char *name)
+uint32_t create_dive_site(const char *name, timestamp_t divetime)
 {
-	struct dive_site *ds = alloc_dive_site(0);
+	uint32_t uuid = create_divesite_uuid(name, divetime);
+	struct dive_site *ds = alloc_dive_site(uuid);
 	ds->name = copy_string(name);
 
-	return ds->uuid;
+	return uuid;
 }
 
 /* same as before, but with GPS data */
-uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude)
+uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude, timestamp_t divetime)
 {
-	struct dive_site *ds = alloc_dive_site(0);
+	uint32_t uuid = create_divesite_uuid(name, divetime);
+	struct dive_site *ds = alloc_dive_site(uuid);
 	ds->name = copy_string(name);
 	ds->latitude = latitude;
 	ds->longitude = longitude;
@@ -231,7 +248,7 @@ void clear_dive_site(struct dive_site *ds)
 	free_taxonomy(&ds->taxonomy);
 }
 
-uint32_t find_or_create_dive_site_with_name(const char *name)
+uint32_t find_or_create_dive_site_with_name(const char *name, timestamp_t divetime)
 {
 	int i;
 	struct dive_site *ds;
@@ -244,5 +261,5 @@ uint32_t find_or_create_dive_site_with_name(const char *name)
 	}
 	if (ds)
 		return ds->uuid;
-	return create_dive_site(name);
+	return create_dive_site(name, divetime);
 }
diff --git a/divesite.h b/divesite.h
index 71f64a0a1..1dd1d04e8 100644
--- a/divesite.h
+++ b/divesite.h
@@ -53,8 +53,8 @@ struct dive_site *alloc_dive_site(uint32_t uuid);
 int nr_of_dives_at_dive_site(uint32_t uuid, bool select_only);
 bool is_dive_site_used(uint32_t uuid, bool select_only);
 void delete_dive_site(uint32_t id);
-uint32_t create_dive_site(const char *name);
-uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude);
+uint32_t create_dive_site(const char *name, timestamp_t divetime);
+uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude, timestamp_t divetime);
 uint32_t get_dive_site_uuid_by_name(const char *name, struct dive_site **dsp);
 uint32_t get_dive_site_uuid_by_gps(degrees_t latitude, degrees_t longitude, struct dive_site **dsp);
 uint32_t get_dive_site_uuid_by_gps_proximity(degrees_t latitude, degrees_t longitude, int distance, struct dive_site **dsp);
@@ -62,7 +62,7 @@ bool dive_site_is_empty(struct dive_site *ds);
 void copy_dive_site(struct dive_site *orig, struct dive_site *copy);
 void clear_dive_site(struct dive_site *ds);
 unsigned int get_distance(degrees_t lat1, degrees_t lon1, degrees_t lat2, degrees_t lon2);
-uint32_t find_or_create_dive_site_with_name(const char *name);
+uint32_t find_or_create_dive_site_with_name(const char *name, timestamp_t divetime);
 
 #define INVALID_DIVE_SITE_NAME "development use only - not a valid dive site name"
 
diff --git a/liquivision.c b/liquivision.c
index add377237..295287c15 100644
--- a/liquivision.c
+++ b/liquivision.c
@@ -101,6 +101,7 @@ static void parse_dives (int log_version, const unsigned char *buf, unsigned int
 
 	while (ptr < buf_size) {
 		int i;
+		bool found_divesite = false;
 		dive = alloc_dive();
 		primary_sensor = 0;
 		dc = &dive->dc;
@@ -148,10 +149,8 @@ static void parse_dives (int log_version, const unsigned char *buf, unsigned int
 		}
 
 		/* Store the location only if we have one */
-		if (len || place_len) {
-			dive->dive_site_uuid = find_or_create_dive_site_with_name(location);
-			free(location);
-		}
+		if (len || place_len)
+			found_divesite = true;
 
 		ptr += len + 4 + place_len;
 
@@ -183,6 +182,12 @@ static void parse_dives (int log_version, const unsigned char *buf, unsigned int
 		dive->when = array_uint32_le(buf + ptr);
 		ptr += 4;
 
+		// now that we have the dive time we can store the divesite
+		// (we need the dive time to create deterministic uuids)
+		if (found_divesite) {
+			dive->dive_site_uuid = find_or_create_dive_site_with_name(location, dive->when);
+			free(location);
+		}
 		//unsigned int end_time = array_uint32_le(buf + ptr);
 		ptr += 4;
 
diff --git a/load-git.c b/load-git.c
index 24fef24de..aa0ef8c2b 100644
--- a/load-git.c
+++ b/load-git.c
@@ -179,7 +179,7 @@ static void parse_dive_gps(char *line, struct membuffer *str, void *_dive)
 	if (!ds) {
 		uuid = get_dive_site_uuid_by_gps(latitude, longitude, NULL);
 		if (!uuid)
-			uuid = create_dive_site_with_gps("", latitude, longitude);
+			uuid = create_dive_site_with_gps("", latitude, longitude, dive->when);
 		dive->dive_site_uuid = uuid;
 	} else {
 		if (dive_site_has_gps_location(ds) &&
@@ -204,7 +204,7 @@ static void parse_dive_location(char *line, struct membuffer *str, void *_dive)
 	if (!ds) {
 		uuid = get_dive_site_uuid_by_name(name, NULL);
 		if (!uuid)
-			uuid = create_dive_site(name);
+			uuid = create_dive_site(name, dive->when);
 		dive->dive_site_uuid = uuid;
 	} else {
 		// we already had a dive site linked to the dive
@@ -1443,8 +1443,8 @@ static int parse_site_entry(git_repository *repo, const git_tree_entry *entry, c
 {
 	if (*suffix == '\0')
 		return report_error("Dive site without uuid");
-	struct dive_site *ds = alloc_dive_site(0);
-	ds->uuid = strtoul(suffix, NULL, 16);
+	uint32_t uuid = strtoul(suffix, NULL, 16);
+	struct dive_site *ds = alloc_dive_site(uuid);
 	git_blob *blob = git_tree_entry_blob(repo, entry);
 	if (!blob)
 		return report_error("Unable to read dive site file");
diff --git a/parse-xml.c b/parse-xml.c
index d8094dbc5..caffd8404 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -990,7 +990,7 @@ static void divinglog_place(char *place, uint32_t *uuid)
 		 country ? country : "");
 	*uuid = get_dive_site_uuid_by_name(buffer, NULL);
 	if (*uuid == 0)
-		*uuid = create_dive_site(buffer);
+		*uuid = create_dive_site(buffer, cur_dive->when);
 
 	city = NULL;
 	country = NULL;
@@ -1137,7 +1137,7 @@ static void gps_lat(char *buffer, struct dive *dive)
 	degrees_t latitude = parse_degrees(buffer, &end);
 	struct dive_site *ds = get_dive_site_for_dive(dive);
 	if (!ds) {
-		dive->dive_site_uuid = create_dive_site_with_gps(NULL, latitude, (degrees_t){0});
+		dive->dive_site_uuid = create_dive_site_with_gps(NULL, latitude, (degrees_t){0}, dive->when);
 	} else {
 		if (ds->latitude.udeg && ds->latitude.udeg != latitude.udeg)
 			fprintf(stderr, "Oops, changing the latitude of existing dive site id %8x name %s; not good\n", ds->uuid, ds->name ?: "(unknown)");
@@ -1151,7 +1151,7 @@ static void gps_long(char *buffer, struct dive *dive)
 	degrees_t longitude = parse_degrees(buffer, &end);
 	struct dive_site *ds = get_dive_site_for_dive(dive);
 	if (!ds) {
-		dive->dive_site_uuid = create_dive_site_with_gps(NULL, (degrees_t){0}, longitude);
+		dive->dive_site_uuid = create_dive_site_with_gps(NULL, (degrees_t){0}, longitude, dive->when);
 	} else {
 		if (ds->longitude.udeg && ds->longitude.udeg != longitude.udeg)
 			fprintf(stderr, "Oops, changing the longitude of existing dive site id %8x name %s; not good\n", ds->uuid, ds->name ?: "(unknown)");
@@ -1189,7 +1189,7 @@ static void gps_in_dive(char *buffer, struct dive *dive)
 			cur_longitude = longitude;
 			dive->dive_site_uuid = uuid;
 		} else {
-			dive->dive_site_uuid = create_dive_site_with_gps("", latitude, longitude);
+			dive->dive_site_uuid = create_dive_site_with_gps("", latitude, longitude, dive->when);
 			ds = get_dive_site_by_uuid(dive->dive_site_uuid);
 		}
 	} else {
@@ -1247,7 +1247,7 @@ static void add_dive_site(char *ds_name, struct dive *dive)
 				ds->name = copy_string(buffer);
 			} else if (!same_string(ds->name, buffer)) {
 				// if it's not the same name, it's not the same dive site
-				dive->dive_site_uuid = create_dive_site(buffer);
+				dive->dive_site_uuid = create_dive_site(buffer, dive->when);
 				struct dive_site *newds = get_dive_site_by_uuid(dive->dive_site_uuid);
 				if (cur_latitude.udeg || cur_longitude.udeg) {
 					// we started this uuid with GPS data, so lets use those
@@ -1263,7 +1263,7 @@ static void add_dive_site(char *ds_name, struct dive *dive)
 				dive->dive_site_uuid = uuid;
 			}
 		} else {
-			dive->dive_site_uuid = create_dive_site(buffer);
+			dive->dive_site_uuid = create_dive_site(buffer, dive->when);
 		}
 	}
 	free(to_free);
@@ -2693,7 +2693,7 @@ extern int cobalt_location(void *handle, int columns, char **data, char **column
 			sprintf(tmp, "%s / %s", location, data[0]);
 			free(location);
 			location = NULL;
-			cur_dive->dive_site_uuid = find_or_create_dive_site_with_name(tmp);
+			cur_dive->dive_site_uuid = find_or_create_dive_site_with_name(tmp, cur_dive->when);
 			free(tmp);
 		} else {
 			location = strdup(data[0]);
@@ -3110,7 +3110,7 @@ extern int divinglog_dive(void *param, int columns, char **data, char **column)
 	cur_dive->when = (time_t)(atol(data[1]));
 
 	if (data[2])
-		cur_dive->dive_site_uuid = find_or_create_dive_site_with_name(data[2]);
+		cur_dive->dive_site_uuid = find_or_create_dive_site_with_name(data[2], cur_dive->when);
 
 	if (data[3])
 		utf8_string(data[3], &cur_dive->buddy);
diff --git a/qt-models/divelocationmodel.cpp b/qt-models/divelocationmodel.cpp
index 5aa3a3ac8..30b3f82ae 100644
--- a/qt-models/divelocationmodel.cpp
+++ b/qt-models/divelocationmodel.cpp
@@ -1,3 +1,4 @@
+#include "units.h"
 #include "divelocationmodel.h"
 #include "dive.h"
 #include <QDebug>
@@ -126,14 +127,14 @@ void LocationInformationModel::update()
 	endResetModel();
 }
 
-int32_t LocationInformationModel::addDiveSite(const QString& name, int lon, int lat)
+int32_t LocationInformationModel::addDiveSite(const QString& name, timestamp_t divetime, int lon, int lat)
 {
 	degrees_t latitude, longitude;
 	latitude.udeg = lat;
 	longitude.udeg = lon;
 
 	beginInsertRows(QModelIndex(), dive_site_table.nr + 2, dive_site_table.nr + 2);
-	uint32_t uuid = create_dive_site_with_gps(name.toUtf8().data(), latitude, longitude);
+	uint32_t uuid = create_dive_site_with_gps(name.toUtf8().data(), latitude, longitude, divetime);
 	qSort(dive_site_table.dive_sites, dive_site_table.dive_sites + dive_site_table.nr, dive_site_less_than);
 	internalRowCount = dive_site_table.nr;
 	endInsertRows();
diff --git a/qt-models/divelocationmodel.h b/qt-models/divelocationmodel.h
index ee52d2ba4..77dbb7bca 100644
--- a/qt-models/divelocationmodel.h
+++ b/qt-models/divelocationmodel.h
@@ -15,7 +15,7 @@ public:
 	int columnCount(const QModelIndex &parent) const;
 	int rowCount(const QModelIndex &parent = QModelIndex()) const;
 	QVariant data(const QModelIndex &index = QModelIndex(), int role = Qt::DisplayRole) const;
-	int32_t addDiveSite(const QString& name, int lat = 0, int lon = 0);
+	int32_t addDiveSite(const QString& name, timestamp_t divetime, int lat = 0, int lon = 0);
 	bool setData(const QModelIndex &index, const QVariant &value, int role);
 	bool removeRows(int row, int count, const QModelIndex & parent = QModelIndex());
 	void setFirstRowTextField(QLineEdit *textField);
diff --git a/qt-ui/maintab.cpp b/qt-ui/maintab.cpp
index 443837a47..c200b36be 100644
--- a/qt-ui/maintab.cpp
+++ b/qt-ui/maintab.cpp
@@ -897,7 +897,7 @@ void MainTab::updateDiveSite(int divenr)
 			cd->dive_site_uuid = pickedUuid;
 		} else if (!newName.isEmpty()) {
 			// user entered a name but didn't pick a dive site, so copy that data
-			uint32_t createdUuid = create_dive_site(displayed_dive_site.name);
+			uint32_t createdUuid = create_dive_site(displayed_dive_site.name, cd->when);
 			struct dive_site *newDs = get_dive_site_by_uuid(createdUuid);
 			copy_dive_site(&displayed_dive_site, newDs);
 			newDs->uuid = createdUuid; // the copy overwrote the uuid
@@ -914,7 +914,7 @@ void MainTab::updateDiveSite(int divenr)
 		} else if (newName != origName) {
 			if (newUuid == 0) {
 				// so we created a new site, add it to the global list
-				uint32_t createdUuid = create_dive_site(displayed_dive_site.name);
+				uint32_t createdUuid = create_dive_site(displayed_dive_site.name, cd->when);
 				struct dive_site *newDs = get_dive_site_by_uuid(createdUuid);
 				copy_dive_site(&displayed_dive_site, newDs);
 				newDs->uuid = createdUuid; // the copy overwrote the uuid
diff --git a/uemis-downloader.c b/uemis-downloader.c
index 17f7129d1..baf948415 100644
--- a/uemis-downloader.c
+++ b/uemis-downloader.c
@@ -803,7 +803,7 @@ static bool process_raw_buffer(device_data_t *devdata, uint32_t deviceid, char *
 			if (for_dive)
 				*for_dive = atoi(val);
 		} else if (!log && dive && !strcmp(tag, "divespot_id")) {
-			dive->dive_site_uuid = create_dive_site("from Uemis");
+			dive->dive_site_uuid = create_dive_site("from Uemis", dive->when);
 			track_divespot(val, dive->dc.diveid, dive->dive_site_uuid);
 		} else if (dive) {
 			parse_tag(dive, tag, val);