GNOME.org
 

[gnome-software/1143-odrs-cannot-be-disabled-by-filling-empty-review-server-setting] odrs: Cannot be disabled by filling empty 'review-server' setting

commit 461fa80063474385405506dc2ea315d23bfcdcb2
Author: Milan Crha <mcrha redhat com>
Date:   Tue Feb 16 16:09:08 2021 +0100

    odrs: Cannot be disabled by filling empty 'review-server' setting
    
    Closes https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1143

 lib/gs-plugin-loader.c        |  22 ++++++++
 lib/gs-plugin.c               |  17 +++++-
 plugins/odrs/gs-plugin-odrs.c | 128 +++++++++++++++++++++++++++++++++---------
 src/gs-details-page.c         |  35 ++++++++++--
 4 files changed, 170 insertions(+), 32 deletions(-)
---
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index e4a034478..6b1a6d621 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -90,6 +90,7 @@ enum {
        SIGNAL_UPDATES_CHANGED,
        SIGNAL_RELOAD,
        SIGNAL_BASIC_AUTH_START,
+       SIGNAL_PLUGIN_ENABLED,
        SIGNAL_LAST
 };
 
@@ -2125,6 +2126,19 @@ gs_plugin_loader_reload_cb (GsPlugin *plugin,
                                       g_object_ref (plugin_loader));
 }
 
+static void
+gs_plugin_loader_notify_enabled_cb (GsPlugin *plugin,
+                                   GParamSpec *param,
+                                   GsPluginLoader *plugin_loader)
+{
+       g_debug ("emitting plugin-enabled for '%s' (enabled:%d)",
+                gs_plugin_get_name (plugin),
+                gs_plugin_get_enabled (plugin));
+       g_signal_emit (plugin_loader,
+                      signals[SIGNAL_PLUGIN_ENABLED],
+                      0, plugin);
+}
+
 static void
 gs_plugin_loader_open_plugin (GsPluginLoader *plugin_loader,
                              const gchar *filename)
@@ -2156,6 +2170,9 @@ gs_plugin_loader_open_plugin (GsPluginLoader *plugin_loader,
        g_signal_connect (plugin, "allow-updates",
                          G_CALLBACK (gs_plugin_loader_allow_updates_cb),
                          plugin_loader);
+       g_signal_connect (plugin, "notify::enabled",
+                         G_CALLBACK (gs_plugin_loader_notify_enabled_cb),
+                         plugin_loader);
        gs_plugin_set_soup_session (plugin, plugin_loader->soup_session);
        gs_plugin_set_locale (plugin, plugin_loader->locale);
        gs_plugin_set_language (plugin, plugin_loader->language);
@@ -2789,6 +2806,11 @@ gs_plugin_loader_class_init (GsPluginLoaderClass *klass)
                              G_TYPE_FROM_CLASS (object_class), G_SIGNAL_RUN_LAST,
                              0, NULL, NULL, g_cclosure_marshal_generic,
                              G_TYPE_NONE, 4, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_POINTER, G_TYPE_POINTER);
+       signals [SIGNAL_PLUGIN_ENABLED] =
+               g_signal_new ("plugin-enabled",
+                             G_TYPE_FROM_CLASS (object_class), G_SIGNAL_RUN_LAST,
+                             0, NULL, NULL, g_cclosure_marshal_VOID__OBJECT,
+                             G_TYPE_NONE, 1, GS_TYPE_PLUGIN);
 }
 
 static void
diff --git a/lib/gs-plugin.c b/lib/gs-plugin.c
index 7c332e6cc..79d9e4ddc 100644
--- a/lib/gs-plugin.c
+++ b/lib/gs-plugin.c
@@ -80,6 +80,7 @@ G_DEFINE_QUARK (gs-plugin-error-quark, gs_plugin_error)
 enum {
        PROP_0,
        PROP_FLAGS,
+       PROP_ENABLED,
        PROP_LAST
 };
 
@@ -353,7 +354,10 @@ void
 gs_plugin_set_enabled (GsPlugin *plugin, gboolean enabled)
 {
        GsPluginPrivate *priv = gs_plugin_get_instance_private (plugin);
-       priv->enabled = enabled;
+       if (priv->enabled != enabled) {
+               priv->enabled = enabled;
+               g_object_notify (G_OBJECT (plugin), "enabled");
+       }
 }
 
 void
@@ -1946,6 +1950,9 @@ gs_plugin_set_property (GObject *object, guint prop_id, const GValue *value, GPa
        GsPlugin *plugin = GS_PLUGIN (object);
        GsPluginPrivate *priv = gs_plugin_get_instance_private (plugin);
        switch (prop_id) {
+       case PROP_ENABLED:
+               gs_plugin_set_enabled (plugin, g_value_get_boolean (value));
+               break;
        case PROP_FLAGS:
                priv->flags = g_value_get_flags (value);
                break;
@@ -1961,6 +1968,9 @@ gs_plugin_get_property (GObject *object, guint prop_id, GValue *value, GParamSpe
        GsPlugin *plugin = GS_PLUGIN (object);
        GsPluginPrivate *priv = gs_plugin_get_instance_private (plugin);
        switch (prop_id) {
+       case PROP_ENABLED:
+               g_value_set_boolean (value, gs_plugin_get_enabled (plugin));
+               break;
        case PROP_FLAGS:
                g_value_set_flags (value, priv->flags);
                break;
@@ -1980,6 +1990,11 @@ gs_plugin_class_init (GsPluginClass *klass)
        object_class->get_property = gs_plugin_get_property;
        object_class->finalize = gs_plugin_finalize;
 
+       pspec = g_param_spec_boolean ("enabled", NULL, NULL,
+                                     TRUE,
+                                     G_PARAM_READWRITE);
+       g_object_class_install_property (object_class, PROP_ENABLED, pspec);
+
        pspec = g_param_spec_flags ("flags", NULL, NULL,
                                    GS_TYPE_PLUGIN_FLAGS, GS_PLUGIN_FLAGS_NONE,
                                    G_PARAM_READWRITE);
diff --git a/plugins/odrs/gs-plugin-odrs.c b/plugins/odrs/gs-plugin-odrs.c
index 5920c4e40..807f5e29a 100644
--- a/plugins/odrs/gs-plugin-odrs.c
+++ b/plugins/odrs/gs-plugin-odrs.c
@@ -119,11 +119,33 @@ struct GsPluginData {
        gchar                   *distro;
        gchar                   *user_hash;
        gchar                   *review_server;
+       GMutex                   review_server_lock;
+       gulong                   review_server_changed_id;
        GArray                  *ratings;  /* (element-type GsOdrsRating) (mutex ratings_mutex) (owned) 
(nullable) */
        GMutex                   ratings_mutex;
        GsApp                   *cached_origin;
 };
 
+static void
+gs_plugin_odrs_review_server_changed_cb (GSettings *settings,
+                                        const gchar *key,
+                                        GsPlugin *plugin)
+{
+       GsPluginData *priv = gs_plugin_get_data (plugin);
+       g_autofree gchar *review_server = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
+
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+       review_server = g_settings_get_string (settings, "review-server");
+       if (g_strcmp0 (review_server, priv->review_server) != 0) {
+               g_free (priv->review_server);
+               priv->review_server = g_steal_pointer (&review_server);
+
+               gs_plugin_set_enabled (plugin, priv->review_server && priv->review_server[0] != '\0');
+               gs_app_set_origin_hostname (priv->cached_origin, priv->review_server);
+       }
+}
+
 void
 gs_plugin_initialize (GsPlugin *plugin)
 {
@@ -132,9 +154,12 @@ gs_plugin_initialize (GsPlugin *plugin)
        g_autoptr(GsOsRelease) os_release = NULL;
 
        g_mutex_init (&priv->ratings_mutex);
+       g_mutex_init (&priv->review_server_lock);
        priv->settings = g_settings_new ("org.gnome.software");
-       priv->review_server = g_settings_get_string (priv->settings,
-                                                    "review-server");
+       priv->review_server_changed_id =
+               g_signal_connect (priv->settings, "changed::review-server",
+                       G_CALLBACK (gs_plugin_odrs_review_server_changed_cb), plugin);
+       priv->review_server = NULL;
        priv->ratings = NULL;  /* until first refreshed */
 
        /* get the machine+user ID hash value */
@@ -160,7 +185,8 @@ gs_plugin_initialize (GsPlugin *plugin)
        /* add source */
        priv->cached_origin = gs_app_new (gs_plugin_get_name (plugin));
        gs_app_set_kind (priv->cached_origin, AS_COMPONENT_KIND_REPOSITORY);
-       gs_app_set_origin_hostname (priv->cached_origin, priv->review_server);
+
+       gs_plugin_odrs_review_server_changed_cb (priv->settings, NULL, plugin);
 
        /* add the source to the plugin cache which allows us to match the
         * unique ID to a GsApp when creating an event */
@@ -277,6 +303,13 @@ gs_plugin_refresh (GsPlugin *plugin,
        g_autofree gchar *uri = NULL;
        g_autoptr(GError) error_local = NULL;
        g_autoptr(GsApp) app_dl = gs_app_new (gs_plugin_get_name (plugin));
+       g_autoptr(GMutexLocker) locker = NULL;
+
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+       if (priv->review_server == NULL || priv->review_server[0] == '\0')
+               return TRUE;
+       uri = g_strdup_printf ("%s/ratings", priv->review_server);
+       g_clear_pointer (&locker, g_mutex_locker_free);
 
        /* check cache age */
        cache_filename = gs_utils_get_cache_filename ("odrs",
@@ -298,7 +331,6 @@ gs_plugin_refresh (GsPlugin *plugin,
        }
 
        /* download the complete file */
-       uri = g_strdup_printf ("%s/ratings", priv->review_server);
        g_debug ("Updating ODRS cache from %s to %s", uri, cache_filename);
        gs_app_set_summary_missing (app_dl,
                                    /* TRANSLATORS: status text when downloading */
@@ -329,8 +361,11 @@ gs_plugin_destroy (GsPlugin *plugin)
        g_free (priv->distro);
        g_free (priv->review_server);
        g_clear_pointer (&priv->ratings, g_array_unref);
+       if (priv->review_server_changed_id)
+               g_signal_handler_disconnect (priv->settings, priv->review_server_changed_id);
        g_object_unref (priv->settings);
        g_object_unref (priv->cached_origin);
+       g_mutex_clear (&priv->review_server_lock);
        g_mutex_clear (&priv->ratings_mutex);
 }
 
@@ -646,6 +681,12 @@ gs_plugin_odrs_refine_ratings (GsPlugin *plugin,
        g_autoptr(GPtrArray) reviewable_ids = NULL;
        g_autoptr(GMutexLocker) locker = NULL;
 
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+       if (priv->review_server == NULL || priv->review_server[0] == '\0')
+               return TRUE;
+
+       g_clear_pointer (&locker, g_mutex_locker_free);
+
        /* get ratings for each reviewable ID */
        reviewable_ids = _gs_app_get_reviewable_ids (app);
 
@@ -764,6 +805,12 @@ gs_plugin_odrs_fetch_for_app (GsPlugin *plugin, GsApp *app, GError **error)
        g_autoptr(JsonGenerator) json_generator = NULL;
        g_autoptr(JsonNode) json_root = NULL;
        g_autoptr(SoupMessage) msg = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
+
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+
+       if (priv->review_server == NULL || priv->review_server[0] == '\0')
+               return g_ptr_array_new ();
 
        /* look in the cache */
        cachefn_basename = g_strdup_printf ("%s.json", gs_app_get_id (app));
@@ -781,6 +828,8 @@ gs_plugin_odrs_fetch_for_app (GsPlugin *plugin, GsApp *app, GError **error)
                if (mapped_file == NULL)
                        return NULL;
 
+               g_clear_pointer (&locker, g_mutex_locker_free);
+
                g_debug ("got review data for %s from %s",
                         gs_app_get_id (app), cachefn);
                return gs_plugin_odrs_parse_reviews (plugin,
@@ -827,6 +876,9 @@ gs_plugin_odrs_fetch_for_app (GsPlugin *plugin, GsApp *app, GError **error)
        uri = g_strdup_printf ("%s/fetch", priv->review_server);
        g_debug ("Updating ODRS cache for %s from %s to %s; request %s", gs_app_get_id (app),
                 uri, cachefn, data);
+
+       g_clear_pointer (&locker, g_mutex_locker_free);
+
        msg = soup_message_new (SOUP_METHOD_POST, uri);
        soup_message_set_request (msg, "application/json; charset=utf-8",
                                  SOUP_MEMORY_COPY, data, strlen (data));
@@ -1030,6 +1082,15 @@ gs_plugin_review_submit (GsPlugin *plugin,
        g_autoptr(JsonBuilder) builder = NULL;
        g_autoptr(JsonGenerator) json_generator = NULL;
        g_autoptr(JsonNode) json_root = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
+
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+
+       if (priv->review_server == NULL || priv->review_server[0] == '\0') {
+               g_set_error_literal (error, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_NOT_SUPPORTED,
+                       "The ODRS plugin is disabled");
+               return FALSE;
+       }
 
        /* save as we don't re-request the review from the server */
        as_review_add_flags (review, AS_REVIEW_FLAG_SELF);
@@ -1079,20 +1140,37 @@ gs_plugin_review_submit (GsPlugin *plugin,
 
        /* POST */
        uri = g_strdup_printf ("%s/submit", priv->review_server);
+       g_clear_pointer (&locker, g_mutex_locker_free);
        return gs_plugin_odrs_json_post (plugin, gs_plugin_get_soup_session (plugin),
                                                    uri, data, error);
 }
 
 static gboolean
-gs_plugin_odrs_vote (GsPlugin *plugin, AsReview *review,
-                    const gchar *uri, GError **error)
+gs_plugin_odrs_vote (GsPlugin *plugin,
+                    AsReview *review,
+                    const gchar *path,
+                    GError **error)
 {
        GsPluginData *priv = gs_plugin_get_data (plugin);
        const gchar *tmp;
        g_autofree gchar *data = NULL;
+       g_autofree gchar *uri = NULL;
        g_autoptr(JsonBuilder) builder = NULL;
        g_autoptr(JsonGenerator) json_generator = NULL;
        g_autoptr(JsonNode) json_root = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
+
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+
+       if (priv->review_server == NULL || priv->review_server[0] == '\0') {
+               g_set_error_literal (error, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_NOT_SUPPORTED,
+                       "The ODRS plugin is disabled");
+               return FALSE;
+       }
+
+       uri = g_strdup_printf ("%s/%s", priv->review_server, path);
+
+       g_clear_pointer (&locker, g_mutex_locker_free);
 
        /* create object with vote data */
        builder = json_builder_new ();
@@ -1147,10 +1225,7 @@ gs_plugin_review_report (GsPlugin *plugin,
                         GCancellable *cancellable,
                         GError **error)
 {
-       GsPluginData *priv = gs_plugin_get_data (plugin);
-       g_autofree gchar *uri = NULL;
-       uri = g_strdup_printf ("%s/report", priv->review_server);
-       return gs_plugin_odrs_vote (plugin, review, uri, error);
+       return gs_plugin_odrs_vote (plugin, review, "report", error);
 }
 
 gboolean
@@ -1160,10 +1235,7 @@ gs_plugin_review_upvote (GsPlugin *plugin,
                         GCancellable *cancellable,
                         GError **error)
 {
-       GsPluginData *priv = gs_plugin_get_data (plugin);
-       g_autofree gchar *uri = NULL;
-       uri = g_strdup_printf ("%s/upvote", priv->review_server);
-       return gs_plugin_odrs_vote (plugin, review, uri, error);
+       return gs_plugin_odrs_vote (plugin, review, "upvote", error);
 }
 
 gboolean
@@ -1173,10 +1245,7 @@ gs_plugin_review_downvote (GsPlugin *plugin,
                           GCancellable *cancellable,
                           GError **error)
 {
-       GsPluginData *priv = gs_plugin_get_data (plugin);
-       g_autofree gchar *uri = NULL;
-       uri = g_strdup_printf ("%s/downvote", priv->review_server);
-       return gs_plugin_odrs_vote (plugin, review, uri, error);
+       return gs_plugin_odrs_vote (plugin, review, "downvote", error);
 }
 
 gboolean
@@ -1186,10 +1255,7 @@ gs_plugin_review_dismiss (GsPlugin *plugin,
                          GCancellable *cancellable,
                          GError **error)
 {
-       GsPluginData *priv = gs_plugin_get_data (plugin);
-       g_autofree gchar *uri = NULL;
-       uri = g_strdup_printf ("%s/dismiss", priv->review_server);
-       return gs_plugin_odrs_vote (plugin, review, uri, error);
+       return gs_plugin_odrs_vote (plugin, review, "dismiss", error);
 }
 
 gboolean
@@ -1199,10 +1265,7 @@ gs_plugin_review_remove (GsPlugin *plugin,
                         GCancellable *cancellable,
                         GError **error)
 {
-       GsPluginData *priv = gs_plugin_get_data (plugin);
-       g_autofree gchar *uri = NULL;
-       uri = g_strdup_printf ("%s/remove", priv->review_server);
-       return gs_plugin_odrs_vote (plugin, review, uri, error);
+       return gs_plugin_odrs_vote (plugin, review, "remove", error);
 }
 
 static GsApp *
@@ -1233,12 +1296,25 @@ gs_plugin_add_unvoted_reviews (GsPlugin *plugin,
        g_autoptr(GPtrArray) reviews = NULL;
        g_autoptr(SoupMessage) msg = NULL;
 
+       g_autoptr(GMutexLocker) locker = NULL;
+
+       locker = g_mutex_locker_new (&priv->review_server_lock);
+
+       if (priv->review_server == NULL || priv->review_server[0] == '\0') {
+               g_set_error_literal (error, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_NOT_SUPPORTED,
+                       "The ODRS plugin is disabled");
+               return FALSE;
+       }
+
        /* create the GET data *with* the machine hash so we can later
         * review the application ourselves */
        uri = g_strdup_printf ("%s/moderate/%s/%s",
                               priv->review_server,
                               priv->user_hash,
                               gs_plugin_get_locale (plugin));
+
+       g_clear_pointer (&locker, g_mutex_locker_free);
+
        msg = soup_message_new (SOUP_METHOD_GET, uri);
        status_code = soup_session_send_message (gs_plugin_get_soup_session (plugin), msg);
        if (status_code != SOUP_STATUS_OK) {
diff --git a/src/gs-details-page.c b/src/gs-details-page.c
index 099ff06d0..31d455c6c 100644
--- a/src/gs-details-page.c
+++ b/src/gs-details-page.c
@@ -53,6 +53,7 @@ struct _GsDetailsPage
        GsApp                   *app_local_file;
        GsShell                 *shell;
        SoupSession             *session;
+       gboolean                 plugin_enabled;
        gboolean                 enable_reviews;
        gboolean                 show_all_reviews;
        GSettings               *settings;
@@ -1549,6 +1550,21 @@ gs_details_page_app_set_review_cb (GObject *source,
        gs_details_page_refresh_reviews (helper->self);
 }
 
+static gboolean
+gs_details_page_get_enable_reviews (GsDetailsPage *self)
+{
+       if (self->plugin_enabled) {
+               self->plugin_enabled = FALSE;
+
+               /* show review widgets if we have plugins that provide them */
+               self->enable_reviews =
+                       gs_plugin_loader_get_plugin_supported (self->plugin_loader,
+                                                              "gs_plugin_review_submit");
+       }
+
+       return self->enable_reviews;
+}
+
 static void
 gs_details_page_review_button_clicked_cb (GsReviewRow *row,
                                           GsPluginAction action,
@@ -1606,7 +1622,7 @@ gs_details_page_refresh_reviews (GsDetailsPage *self)
        case AS_COMPONENT_KIND_WEB_APP:
                /* don't show a missing rating on a local file */
                if (gs_app_get_state (self->app) != GS_APP_STATE_AVAILABLE_LOCAL &&
-                   self->enable_reviews)
+                   gs_details_page_get_enable_reviews (self))
                        show_reviews = TRUE;
                break;
        default:
@@ -2688,6 +2704,14 @@ gs_details_page_network_available_notify_cb (GsPluginLoader *plugin_loader,
        gs_details_page_refresh_reviews (self);
 }
 
+static void
+gs_details_page_plugin_enabled_cb (GsPluginLoader *plugin_loader,
+                                  GsPlugin *plugin,
+                                  GsDetailsPage *self)
+{
+       self->plugin_enabled = TRUE;
+}
+
 static void
 gs_details_page_star_pressed_cb(GtkWidget *widget, GdkEventButton *event, GsDetailsPage *self)
 {
@@ -2713,11 +2737,8 @@ gs_details_page_setup (GsPage *page,
        self->plugin_loader = g_object_ref (plugin_loader);
        self->builder = g_object_ref (builder);
        self->cancellable = g_object_ref (cancellable);
+       self->plugin_enabled = TRUE;
 
-       /* show review widgets if we have plugins that provide them */
-       self->enable_reviews =
-               gs_plugin_loader_get_plugin_supported (plugin_loader,
-                                                      "gs_plugin_review_submit");
        g_signal_connect (self->button_review, "clicked",
                          G_CALLBACK (gs_details_page_write_review_cb),
                          self);
@@ -2730,6 +2751,10 @@ gs_details_page_setup (GsPage *page,
                                 G_CALLBACK (gs_details_page_network_available_notify_cb),
                                 self, 0);
 
+       g_signal_connect_object (self->plugin_loader, "plugin-enabled",
+                                G_CALLBACK (gs_details_page_plugin_enabled_cb),
+                                self, 0);
+
        /* setup details */
        g_signal_connect (self->button_install, "clicked",
                          G_CALLBACK (gs_details_page_app_install_button_cb),
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]