Fix a very annoying logic bug in URL insertion

Write test case for it

internal/database/actions.go

@@ -202,48 +202,48 @@ func CreateNetwork(sess sqlbuilder.Database, location *utils.LocationInfo) (*Net
 // CreateOrUpdateURL will create a new URL entry to the urls table if it doesn't
 // exists, otherwise it will update the category code of the one already in
 // there.
-func CreateOrUpdateURL(sess sqlbuilder.Database, url string, categoryCode string, countryCode string) (int64, error) {
-	var urlID int64
-
-	res, err := sess.Update("urls").Set(
-		"url", url,
-		"category_code", categoryCode,
-		"country_code", countryCode,
-	).Where("url = ? AND country_code = ?", url, countryCode).Exec()
+func CreateOrUpdateURL(sess sqlbuilder.Database, urlStr string, categoryCode string, countryCode string) (int64, error) {
+	var url URL
 
+	tx, err := sess.NewTx(nil)
 	if err != nil {
-		log.Error("Failed to write to the URL table")
+		log.WithError(err).Error("failed to create transaction")
 		return 0, err
 	}
-	affected, err := res.RowsAffected()
+	res := tx.Collection("urls").Find(
+		db.Cond{"url": urlStr, "country_code": countryCode},
+	)
+	err = res.One(&url)
 
-	if err != nil {
-		log.Error("Failed to get affected row count")
-		return 0, err
-	}
-	if affected == 0 {
-		newID, err := sess.Collection("urls").Insert(
-			URL{
-				URL:          sql.NullString{String: url, Valid: true},
+	if err == db.ErrNoMoreRows {
+		url = URL{
+			URL:          sql.NullString{String: urlStr, Valid: true},
 			CategoryCode: sql.NullString{String: categoryCode, Valid: true},
 			CountryCode:  sql.NullString{String: countryCode, Valid: true},
-			})
-		if err != nil {
+		}
+		newID, insErr := tx.Collection("urls").Insert(url)
+		if insErr != nil {
 			log.Error("Failed to insert into the URLs table")
-			return 0, err
+			return 0, insErr
 		}
-		urlID = newID.(int64)
+		url.ID = sql.NullInt64{Int64: newID.(int64), Valid: true}
+	} else if err != nil {
+		log.WithError(err).Error("Failed to get single result")
+		return 0, err
 	} else {
-		lastID, err := res.LastInsertId()
+		url.CategoryCode = sql.NullString{String: categoryCode, Valid: true}
+		res.Update(url)
+	}
+
+	err = tx.Commit()
 	if err != nil {
-			log.Error("failed to get URL ID")
+		log.WithError(err).Error("Failed to write to the URL table")
 		return 0, err
 	}
-		urlID = lastID
-	}
-	log.Debugf("returning url %d", urlID)
 
-	return urlID, nil
+	log.Debugf("returning url %d", url.ID.Int64)
+
+	return url.ID.Int64, nil
 }
 
 // AddTestKeys writes the summary to the measurement

internal/database/actions_test.go

@@ -132,12 +132,27 @@ func TestURLCreation(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	newID1, err := CreateOrUpdateURL(sess, "https://google.com", "SRCH", "XX")
+	newID1, err := CreateOrUpdateURL(sess, "https://google.com", "GMB", "XX")
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	newID2, err := CreateOrUpdateURL(sess, "https://google.com", "GMB", "XX")
+	newID2, err := CreateOrUpdateURL(sess, "https://google.com", "SRCH", "XX")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newID3, err := CreateOrUpdateURL(sess, "https://facebook.com", "GRP", "XX")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newID4, err := CreateOrUpdateURL(sess, "https://facebook.com", "GMP", "XX")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newID5, err := CreateOrUpdateURL(sess, "https://google.com", "SRCH", "XX")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -145,6 +160,18 @@ func TestURLCreation(t *testing.T) {
 	if newID2 != newID1 {
 		t.Error("inserting the same URL with different category code should produce the same result")
 	}
+
+	if newID3 == newID1 {
+		t.Error("inserting different URL should produce different ids")
+	}
+
+	if newID4 != newID3 {
+		t.Error("inserting the same URL with different category code should produce the same result")
+	}
+
+	if newID5 != newID1 {
+		t.Error("the ID of google should still be the same")
+	}
 }
 
 func TestPerformanceTestKeys(t *testing.T) {

nettests/nettests.go

@@ -176,12 +176,12 @@ func (c *Controller) Init(nt *mk.Nettest) error {
 
 	nt.On("status.measurement_start", func(e mk.Event) {
 		log.Debugf(color.RedString(e.Key))
-
 		idx := e.Value.Idx
 		urlID := sql.NullInt64{Int64: 0, Valid: false}
 		if c.inputIdxMap != nil {
 			urlID = sql.NullInt64{Int64: c.inputIdxMap[idx], Valid: true}
 		}
+		log.Debugf("👁 %d %s %d", idx, e.Value.Input, urlID.Int64)
 		msmt, err := database.CreateMeasurement(c.Ctx.DB, reportID, testName, resultID, reportFilePath, urlID)
 		if err != nil {
 			log.WithError(err).Error("Failed to create measurement")

nettests/websites/web_connectivity.go

@@ -59,6 +59,7 @@ func lookupURLs(ctl *nettests.Controller) ([]string, map[int64]int64, error) {
 		if err != nil {
 			log.Error("failed to add to the URL table")
 		}
+		log.Debugf("Mapped URL %s to idx %d and urlID %d", url.URL, idx, urlID)
 		urlIDMap[int64(idx)] = urlID
 		urls = append(urls, url.URL)
 	}