mariadb: update UpdateMetadata

This changes the linkage from:
	new.hash_link > old.hash
to
	old.hash_link > new.hash

we still need to make it update all old hashes to point to the new
one if applicable but that's for later after tests are written
to test the edge cases

storage/mariadb/track.go

@@ -739,7 +739,8 @@ UPDATE tracks SET
 	path=:filepath,
 	hash=:hash,
 	tags=:tags,
-	need_reupload=:needreplacement
+	need_reupload=:needreplacement,
+	lasteditor=:lasteditor
 WHERE id=:trackid;
 `
 
@@ -763,30 +764,28 @@ func (ts TrackStorage) UpdateMetadata(song radio.Song) error {
 	}
 	defer tx.Rollback()
 
-	// we need to update the song hash, this might not have actually changed but
-	// we do this unconditionally since we don't know if they changed or not until
-	// we've recalculated the hash
+	// since a metadata update means a hash update we recalculate the hash here
+	// to see if either Artist or Title has changed
 	oldHash := song.Hash
-	song.Metadata = radio.Metadata(song.Artist, song.Title)
-	song.Hash = radio.NewSongHash(song.Metadata)
+
+	// set the metadata to nothing so Hydrate will fill it in for us
+	song.Metadata = ""
+	song.Hydrate()
 
 	if song.Hash != oldHash {
-		// hash has changed so we need to potentially create an song entry too if
-		// this is a non-existant hash
+		// hash is different, so make sure we have a song entry for it
 		otherSong, err := SongStorage{handle}.Create(song)
 		if err != nil {
 			return errors.E(op, err, song)
 		}
+		// update the song entries ID since we might've just created a new one
+		song.ID = otherSong.ID
 
-		// check if the song we got back has a hash_link set, if it does it means it was
-		// already renamed once and we don't touch it any further.
-		if otherSong.Hash == otherSong.HashLink {
-			// If they're still the same it means there are no other entries linked
-			// we can change it to our old hash so that favorites and plays transfer over
-			err := SongStorage{handle}.UpdateHashLink(otherSong.Hash, oldHash)
-			if err != nil {
-				return errors.E(op, err)
-			}
+		// update the old song entry's hash link to point to our newly created song
+		// TODO: also update the whole graph
+		err = SongStorage{handle}.UpdateHashLink(oldHash, song.Hash)
+		if err != nil {
+			return errors.E(op, err)
 		}
 	}
 

storage/test/track.go

@@ -118,3 +118,53 @@ func (suite *Suite) TestSongLastPlayed(t *testing.T) {
 		assert.True(t, original.EqualTo(lp[i]), "subset end: expected %s got %s", original.Metadata, lp[i].Metadata)
 	}
 }
+
+func (suite *Suite) TestTrackUpdateMetadata(t *testing.T) {
+	s := suite.Storage(t)
+	ts := s.Track(suite.ctx)
+	ss := s.Song(suite.ctx)
+
+	original := radio.Song{
+		DatabaseTrack: &radio.DatabaseTrack{
+			Artist:     "artist test",
+			Album:      "album test",
+			Title:      "title test",
+			Acceptor:   "test user",
+			LastEditor: "test user",
+		},
+	}
+	original.Hydrate()
+
+	new, err := ts.Insert(original)
+	require.NoError(t, err)
+	require.NotZero(t, new)
+
+	updated := original
+	updated.DatabaseTrack = &radio.DatabaseTrack{
+		TrackID:    new,
+		Artist:     "new artist",
+		Album:      original.Album,
+		Title:      original.Title,
+		Acceptor:   original.Acceptor,
+		LastEditor: "some other user",
+	}
+
+	err = ts.UpdateMetadata(updated)
+	require.NoError(t, err)
+
+	// we can now get an updated version with all fields we care about updated from the db
+	updatedSong, err := ts.Get(new)
+	require.NoError(t, err)
+	require.NotNil(t, updatedSong)
+
+	// and the old song entry from before we updated
+	originalSong, err := ss.FromHash(original.Hash)
+	require.NoError(t, err)
+	require.NotNil(t, originalSong)
+
+	assert.Equal(t, updatedSong.Hash.String(), originalSong.HashLink.String(),
+		"original song entry's hashlink should be pointing to the updated hash")
+	assert.Equal(t, updatedSong.Artist, updated.Artist)
+	assert.Equal(t, updatedSong.Album, updated.Album)
+	assert.Equal(t, updatedSong.Title, updated.Title)
+}