Rework ff98a7907c to use the newly added EntryId column rather than deleting based on the start time

backend/server/internal/database/db.go

@@ -152,14 +152,8 @@ func (db *DB) DumpRequestDeleteForUserAndDevice(ctx context.Context, userID, dev
 func (db *DB) ApplyDeletionRequestsToBackend(ctx context.Context, request *shared.DeletionRequest) (int64, error) {
 	tx := db.WithContext(ctx).Where("false")
 	for _, message := range request.Messages.Ids {
-		// Note that this won't do server-side deletion of pre-saved history entries. This is an inherent
-		// limitation of our current DB schema. This is sub-par, since it means that even after deletion, clients
-		// may still receive deleted history entries. But, a well-behaved client will immediately delete
-		// these (never writing them to disk) and mark them as received, so this won't happen again.
-		//
-		// TODO: This could be improved upon if we added a HistoryEntry.EntryId field, backfilled it, added
-		// it to EncHistoryEntry, and then used that as a key for deletion.
-		tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND date = ?", request.UserId, message.DeviceId, message.EndTime))
+		// Note that we do an OR with date or the ID matching since the ID is not always recorded for older history entries.
+		tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND (date = ? OR id = ?)", request.UserId, message.DeviceId, message.EndTime, message.EntryId))
 	}
 	result := tx.Delete(&shared.EncHistoryEntry{})
 	if tx.Error != nil {

client/cmd/redact.go

@@ -87,7 +87,7 @@ func deleteOnRemoteInstances(ctx context.Context, historyEntries []*data.History
 
 	for _, entry := range historyEntries {
 		deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids,
-			shared.MessageIdentifier{StartTime: entry.StartTime, EndTime: entry.EndTime, DeviceId: entry.DeviceId},
+			shared.MessageIdentifier{DeviceId: entry.DeviceId, EndTime: entry.EndTime, EntryId: entry.EntryId},
 		)
 	}
 	return lib.SendDeletionRequest(deletionRequest)

client/lib/lib.go

@@ -605,9 +605,9 @@ func HandleDeletionRequests(ctx context.Context, deletionRequests []*shared.Dele
 	db := hctx.GetDb(ctx)
 	for _, request := range deletionRequests {
 		for _, entry := range request.Messages.Ids {
-			// Note that entry.StartTime is not always present (for legacy reasons) and entry.EndTime is also
-			// not always present (for pre-saved entries). So we just check that one of them matches.
-			tx := db.Where("device_id = ? AND (start_time = ? OR end_time = ?)", entry.DeviceId, entry.StartTime, entry.EndTime)
+			// Note that entry.EndTime is not always present (for pre-saved entries). And likewise,
+			// entry.EntryId is not always present for older entries. So we just check that one of them matches.
+			tx := db.Where("device_id = ? AND (end_time = ? OR entry_id = ?)", entry.DeviceId, entry.EndTime, entry.EntryId)
 			res := tx.Delete(&data.HistoryEntry{})
 			if res.Error != nil {
 				return fmt.Errorf("DB error: %w", res.Error)

client/tui/tui.go

@@ -602,7 +602,7 @@ func deleteHistoryEntry(ctx context.Context, entry data.HistoryEntry) error {
 		SendTime: time.Now(),
 	}
 	dr.Messages.Ids = append(dr.Messages.Ids,
-		shared.MessageIdentifier{StartTime: entry.StartTime, EndTime: entry.EndTime, DeviceId: entry.DeviceId},
+		shared.MessageIdentifier{DeviceId: entry.DeviceId, EndTime: entry.EndTime, EntryId: entry.EntryId},
 	)
 	return lib.SendDeletionRequest(dr)
 }

shared/data.go

@@ -14,13 +14,15 @@ type EncHistoryEntry struct {
 	DeviceId      string `json:"device_id"`
 	UserId        string `json:"user_id"`
 	// Note that EncHistoryEntry.Date == HistoryEntry.EndTime
-	Date        time.Time `json:"time"`
-	EncryptedId string    `json:"id"`
-	ReadCount   int       `json:"read_count"`
+	Date time.Time `json:"time"`
+	// Note that EncHistoryEntry.EncryptedId == HistoryEntry.Id (for entries created after pre-saving support)
+	EncryptedId string `json:"encrypted_id"`
+	ReadCount   int    `json:"read_count"`
 }
 
 /*
 Manually created the indices:
+CREATE INDEX CONCURRENTLY entry_id_idx ON enc_history_entries USING btree(encrypted_id);
 CREATE INDEX CONCURRENTLY device_id_idx ON enc_history_entries USING btree(device_id);
 CREATE INDEX CONCURRENTLY read_count_idx ON enc_history_entries USING btree(read_count);
 CREATE INDEX CONCURRENTLY redact_idx ON enc_history_entries USING btree(user_id, device_id, date);
@@ -82,7 +84,7 @@ type MessageIdentifiers struct {
 	Ids []MessageIdentifier `json:"message_ids"`
 }
 
-// Identifies a single history entry based on the device that recorded the entry, and the end time. Note that
+// Identifies a single history entry based on the device that recorded the entry, and additional metadata. Note that
 // this does not include the command itself since that would risk including the sensitive data that is meant
 // to be deleted
 type MessageIdentifier struct {
@@ -90,9 +92,11 @@ type MessageIdentifier struct {
 	DeviceId string `json:"device_id"`
 	// The timestamp when the command finished running. Serialized as "date" for legacy compatibility.
 	EndTime time.Time `json:"date"`
-	// The timestamp when the command started running.
+	// The entry ID of the command.
 	// Note this field was added as part of supporting pre-saving commands, so older clients do not set this field
-	StartTime time.Time `json:"start_time"`
+	// And even for new clients, it may contain a per-device entry ID. For pre-saved entries, this is guaranteed to
+	// be present.
+	EntryId string `json:"entry_id"`
 }
 
 func (m *MessageIdentifiers) Scan(value interface{}) error {