From a08f8b229c82fbb4b5832ea667dcae0a834a77db Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Mon, 21 Nov 2022 10:55:29 +0000 Subject: [PATCH] Only modify the historical metadata if a release is available This was causing the SystemIntegrityOld metadata key to be overwritten and not included in the upload. It is much better to be explicit rather than using the default release, which may not be what the user actually upgraded to. --- src/fu-engine.c | 15 +++-- src/fu-history.c | 139 +++++++++++++++++++++++++---------------------- src/fu-history.h | 8 +-- 3 files changed, 87 insertions(+), 75 deletions(-) diff --git a/src/fu-engine.c b/src/fu-engine.c index 9c2a7d33a..3b319c1ea 100644 --- a/src/fu-engine.c +++ b/src/fu-engine.c @@ -7318,10 +7318,10 @@ fu_engine_update_history_device(FuEngine *self, FuDevice *dev_history, GError ** metadata_device = fu_device_report_metadata_post(dev); if (metadata_device != NULL && g_hash_table_size(metadata_device) > 0) { fwupd_release_add_metadata(rel_history, metadata_device); - if (!fu_history_set_device_metadata(self->history, - fu_device_get_id(dev_history), - fwupd_release_get_metadata(rel_history), - error)) { + if (!fu_history_modify_device_release(self->history, + dev_history, + rel_history, + error)) { g_prefix_error(error, "failed to set metadata: "); return FALSE; } @@ -7353,7 +7353,10 @@ fu_engine_update_history_device(FuEngine *self, FuDevice *dev_history, GError ** fu_device_set_version(dev_history, fu_device_get_version(dev)); fu_device_remove_flag(dev_history, FWUPD_DEVICE_FLAG_NEEDS_ACTIVATION); fu_device_set_update_state(dev_history, FWUPD_UPDATE_STATE_SUCCESS); - return fu_history_modify_device(self->history, dev_history, error); + return fu_history_modify_device_release(self->history, + dev_history, + rel_history, + error); } /* does the plugin know the update failure */ @@ -7372,7 +7375,7 @@ fu_engine_update_history_device(FuEngine *self, FuDevice *dev_history, GError ** } /* update the state in the database */ - return fu_history_modify_device(self->history, dev_history, error); + return fu_history_modify_device_release(self->history, dev_history, rel_history, error); } static gboolean diff --git a/src/fu-history.c b/src/fu-history.c index 61060d24e..4f2d1735d 100644 --- a/src/fu-history.c +++ b/src/fu-history.c @@ -581,7 +581,80 @@ gboolean fu_history_modify_device(FuHistory *self, FuDevice *device, GError **error) { #ifdef HAVE_SQLITE - FwupdRelease *release; + gint rc; + g_autoptr(sqlite3_stmt) stmt = NULL; + g_autoptr(GRWLockWriterLocker) locker = NULL; + + g_return_val_if_fail(FU_IS_HISTORY(self), FALSE); + g_return_val_if_fail(FU_IS_DEVICE(device), FALSE); + + /* lazy load */ + if (!fu_history_load(self, error)) + return FALSE; + + /* overwrite entry if it exists */ + locker = g_rw_lock_writer_locker_new(&self->db_mutex); + g_return_val_if_fail(locker != NULL, FALSE); + g_debug("modifying device %s [%s]", fu_device_get_name(device), fu_device_get_id(device)); + rc = sqlite3_prepare_v2(self->db, + "UPDATE history SET " + "update_state = ?1, " + "update_error = ?2, " + "checksum_device = ?6, " + "device_modified = ?7, " + "flags = ?3 " + "WHERE device_id = ?4;", + -1, + &stmt, + NULL); + if (rc != SQLITE_OK) { + g_set_error(error, + FWUPD_ERROR, + FWUPD_ERROR_INTERNAL, + "Failed to prepare SQL to update history: %s", + sqlite3_errmsg(self->db)); + return FALSE; + } + + sqlite3_bind_int(stmt, 1, fu_device_get_update_state(device)); + sqlite3_bind_text(stmt, 2, fu_device_get_update_error(device), -1, SQLITE_STATIC); + sqlite3_bind_int64(stmt, 3, fu_history_get_device_flags_filtered(device)); + sqlite3_bind_text(stmt, 4, fu_device_get_id(device), -1, SQLITE_STATIC); + sqlite3_bind_text(stmt, 5, fu_device_get_version(device), -1, SQLITE_STATIC); + sqlite3_bind_text( + stmt, + 6, + fwupd_checksum_get_by_kind(fu_device_get_checksums(device), G_CHECKSUM_SHA1), + -1, + SQLITE_STATIC); + sqlite3_bind_int64(stmt, 7, fu_device_get_modified(device)); + + return fu_history_stmt_exec(self, stmt, NULL, error); +#else + return TRUE; +#endif +} + +/** + * fu_history_modify_device_release: + * @self: a #FuHistory + * @device: a #FuDevice + * @release: a #FwupdRelease + * @error: (nullable): optional return location for an error + * + * Modify a device in the history database, also changing metadata from the new release. + * + * Returns: @TRUE if successful, @FALSE for failure + * + * Since: 1.8.8 + **/ +gboolean +fu_history_modify_device_release(FuHistory *self, + FuDevice *device, + FwupdRelease *release, + GError **error) +{ +#ifdef HAVE_SQLITE gint rc; g_autofree gchar *metadata = NULL; g_autoptr(sqlite3_stmt) stmt = NULL; @@ -595,7 +668,6 @@ fu_history_modify_device(FuHistory *self, FuDevice *device, GError **error) return FALSE; /* metadata is stored as a simple string */ - release = fu_device_get_release_default(device); metadata = _convert_hash_to_string(fwupd_release_get_metadata(release)); /* overwrite entry if it exists */ @@ -643,69 +715,6 @@ fu_history_modify_device(FuHistory *self, FuDevice *device, GError **error) #endif } -/** - * fu_history_set_device_metadata: - * @self: a #FuHistory - * @device_id: a DeviceID string - * @metadata: a #GHashTable of string:string - * @error: (nullable): optional return location for an error - * - * Modify a device in the history database - * - * Returns: @TRUE if successful, @FALSE for failure - * - * Since: 1.5.0 - **/ -gboolean -fu_history_set_device_metadata(FuHistory *self, - const gchar *device_id, - GHashTable *metadata, - GError **error) -{ -#ifdef HAVE_SQLITE - gint rc; - g_autofree gchar *metadata_str = NULL; - g_autoptr(GRWLockWriterLocker) locker = NULL; - g_autoptr(sqlite3_stmt) stmt = NULL; - - g_return_val_if_fail(FU_IS_HISTORY(self), FALSE); - g_return_val_if_fail(device_id != NULL, FALSE); - - /* lazy load */ - if (!fu_history_load(self, error)) - return FALSE; - - /* overwrite entry if it exists */ - locker = g_rw_lock_writer_locker_new(&self->db_mutex); - g_return_val_if_fail(locker != NULL, FALSE); - g_debug("modifying %s", device_id); - rc = sqlite3_prepare_v2(self->db, - "UPDATE history SET " - "metadata = ?1 " - "WHERE device_id = ?2;", - -1, - &stmt, - NULL); - if (rc != SQLITE_OK) { - g_set_error(error, - FWUPD_ERROR, - FWUPD_ERROR_INTERNAL, - "failed to prepare SQL to update history: %s", - sqlite3_errmsg(self->db)); - return FALSE; - } - - /* metadata is stored as a simple string */ - metadata_str = _convert_hash_to_string(metadata); - sqlite3_bind_text(stmt, 1, metadata_str, -1, SQLITE_STATIC); - sqlite3_bind_text(stmt, 2, device_id, -1, SQLITE_STATIC); - - return fu_history_stmt_exec(self, stmt, NULL, error); -#else - return TRUE; -#endif -} - /** * fu_history_add_device: * @self: a #FuHistory diff --git a/src/fu-history.h b/src/fu-history.h index b7442e335..d584ec14f 100644 --- a/src/fu-history.h +++ b/src/fu-history.h @@ -19,10 +19,10 @@ fu_history_add_device(FuHistory *self, FuDevice *device, FwupdRelease *release, gboolean fu_history_modify_device(FuHistory *self, FuDevice *device, GError **error); gboolean -fu_history_set_device_metadata(FuHistory *self, - const gchar *device_id, - GHashTable *metadata, - GError **error); +fu_history_modify_device_release(FuHistory *self, + FuDevice *device, + FwupdRelease *release, + GError **error); gboolean fu_history_remove_device(FuHistory *self, FuDevice *device, GError **error); gboolean