diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 313972d..f524520 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,4 +40,4 @@ jobs: - name: Build run: ninja -C _build - name: Run tests - run: meson test -C _build + run: meson test -C _build --print-errorlogs diff --git a/daemon/emer-daemon.c b/daemon/emer-daemon.c index 037499c..9cb73ea 100644 --- a/daemon/emer-daemon.c +++ b/daemon/emer-daemon.c @@ -150,8 +150,6 @@ struct _EmerDaemon GRand *rand; - gchar *server_url; - guint upload_events_timeout_source_id; guint report_invalid_cache_data_source_id; guint dispatch_aggregate_timers_daily_source_id; @@ -175,7 +173,6 @@ enum { PROP_0, PROP_RANDOM_NUMBER_GENERATOR, - PROP_SERVER_URL, PROP_NETWORK_SEND_INTERVAL, PROP_PERMISSIONS_PROVIDER, PROP_PERSISTENT_CACHE_DIRECTORY, @@ -361,10 +358,12 @@ get_http_request_url (EmerDaemon *self, const guchar *data, gsize length) { + g_autofree gchar *server_url = + emer_permissions_provider_get_server_url (self->permissions_provider); gchar *checksum = g_compute_checksum_for_data (G_CHECKSUM_SHA512, data, length); gchar *http_request_url_string = - g_build_filename (self->server_url, checksum, NULL); + g_build_filename (server_url, CLIENT_VERSION_NUMBER, checksum, NULL); g_free (checksum); g_autoptr(GError) error = NULL; @@ -552,12 +551,17 @@ handle_http_response (GObject *source_object, flush_to_persistent_cache (self); } + /* Log URL without checksum */ + GUri *url = soup_message_get_uri (http_message); + g_autoptr(GUri) base = g_uri_parse_relative (url, ".", G_URI_FLAGS_NONE, &error); + g_autofree gchar *base_str = g_uri_to_string (base); + g_message ("Uploaded " "%" G_GSIZE_FORMAT " events from persistent cache, " "%" G_GSIZE_FORMAT " events from buffer to %s.", callback_data->num_stored_events, callback_data->num_buffer_events, - self->server_url); + base_str); g_task_return_boolean (upload_task, TRUE); finish_network_callback (upload_task); return; @@ -835,13 +839,16 @@ handle_network_monitor_can_reach (GNetworkMonitor *network_monitor, static GSocketConnectable * get_ping_socket (EmerDaemon *self) { + g_autofree gchar *server_url = + emer_permissions_provider_get_server_url (self->permissions_provider); GError *error = NULL; GSocketConnectable *ping_socket = - g_network_address_parse_uri (self->server_url, 443 /* SSL default port */, + g_network_address_parse_uri (server_url, + 443 /* SSL default port */, &error); if (ping_socket == NULL) g_error ("Invalid server URL '%s' could not be parsed because: %s.", - self->server_url, error->message); + server_url, error->message); return ping_socket; } @@ -889,8 +896,7 @@ upload_permitted (EmerDaemon *self, } static void -dequeue_and_do_upload (EmerDaemon *self, - const gchar *environment) +dequeue_and_do_upload (EmerDaemon *self) { GError *error = NULL; if (!upload_permitted (self, &error)) @@ -902,16 +908,6 @@ dequeue_and_do_upload (EmerDaemon *self, return; } - g_assert (self->server_url != NULL); - if (strstr (self->server_url, "${environment}") != NULL) - { - GString *s = g_string_new (self->server_url); - g_string_replace (s, "${environment}", environment, 0); - - g_free (self->server_url); - self->server_url = g_string_free (s, FALSE); - } - GNetworkMonitor *network_monitor = g_network_monitor_get_default (); GSocketConnectable *ping_socket = get_ping_socket (self); g_network_monitor_can_reach_async (network_monitor, ping_socket, NULL, @@ -923,7 +919,6 @@ dequeue_and_do_upload (EmerDaemon *self, static void upload_events (EmerDaemon *self, gsize max_upload_size, - const gchar *environment, GAsyncReadyCallback callback, gpointer user_data) { @@ -935,7 +930,7 @@ upload_events (EmerDaemon *self, g_task_set_task_data (upload_task, callback_data, (GDestroyNotify) network_callback_data_free); g_queue_push_tail (self->upload_queue, upload_task); - dequeue_and_do_upload (self, environment); + dequeue_and_do_upload (self); } static void @@ -963,7 +958,7 @@ handle_upload_timer (EmerDaemon *self) gchar *environment = emer_permissions_provider_get_environment (self->permissions_provider); schedule_upload (self, environment); - upload_events (self, MAX_REQUEST_PAYLOAD, environment, + upload_events (self, MAX_REQUEST_PAYLOAD, (GAsyncReadyCallback) log_upload_error, NULL /* user_data */); g_free (environment); @@ -976,10 +971,7 @@ handle_upload_finished (EmerDaemon *self) if (g_queue_is_empty (self->upload_queue)) return; - gchar *environment = - emer_permissions_provider_get_environment (self->permissions_provider); - dequeue_and_do_upload (self, environment); - g_free (environment); + dequeue_and_do_upload (self); } static void @@ -987,6 +979,8 @@ on_permissions_changed (EmerPermissionsProvider *permissions_provider, GParamSpec *pspec, EmerDaemon *self) { + g_assert (EMER_IS_DAEMON (self)); + self->recording_enabled = emer_permissions_provider_get_daemon_enabled (permissions_provider); @@ -1029,17 +1023,6 @@ set_random_number_generator (EmerDaemon *self, self->rand = rand == NULL ? g_rand_new () : rand; } -static void -set_server_url (EmerDaemon *self, - const gchar *server_url) -{ - g_free (self->server_url); - self->server_url = NULL; - - if (server_url != NULL) - self->server_url = g_build_filename (server_url, CLIENT_VERSION_NUMBER "/", NULL); -} - static void set_network_send_interval (EmerDaemon *self, guint seconds) @@ -1056,8 +1039,11 @@ set_permissions_provider (EmerDaemon *self, else self->permissions_provider = g_object_ref (permissions_provider); - g_signal_connect (self->permissions_provider, "notify::daemon-enabled", - G_CALLBACK (on_permissions_changed), self); + g_signal_connect_object (self->permissions_provider, + "notify::daemon-enabled", + G_CALLBACK (on_permissions_changed), + G_OBJECT (self), + G_CONNECT_DEFAULT); self->recording_enabled = emer_permissions_provider_get_daemon_enabled (self->permissions_provider); } @@ -1394,15 +1380,6 @@ emer_daemon_constructed (GObject *object) } buffer_past_aggregate_events (self); - if (self->server_url == NULL) - { - g_autofree gchar *server_url = emer_permissions_provider_get_server_url (self->permissions_provider); - if (server_url != NULL) - set_server_url (self, server_url); - else - set_server_url (self, DEFAULT_METRICS_SERVER_URL); - } - gchar *environment = emer_permissions_provider_get_environment (self->permissions_provider); schedule_upload (self, environment); @@ -1448,10 +1425,6 @@ emer_daemon_set_property (GObject *object, set_random_number_generator (self, g_value_get_pointer (value)); break; - case PROP_SERVER_URL: - set_server_url (self, g_value_get_string (value)); - break; - case PROP_NETWORK_SEND_INTERVAL: set_network_send_interval (self, g_value_get_uint (value)); break; @@ -1513,7 +1486,6 @@ emer_daemon_finalize (GObject *object) g_clear_pointer (&self->variant_array, g_ptr_array_unref); g_rand_free (self->rand); - g_clear_pointer (&self->server_url, g_free); g_clear_object (&self->permissions_provider); g_clear_object (&self->aggregate_tally); g_clear_pointer (&self->persistent_cache_directory, g_free); @@ -1547,20 +1519,6 @@ emer_daemon_class_init (EmerDaemonClass *klass) G_PARAM_CONSTRUCT_ONLY | G_PARAM_WRITABLE | G_PARAM_STATIC_STRINGS); - /* - * EmerDaemon:server-url: - * - * The URL to which events are uploaded. The URL must contain the protocol and - * may contain the port number. If unspecified, the port number defaults to - * 443, which is the standard port number for SSL. - */ - emer_daemon_props[PROP_SERVER_URL] = - g_param_spec_string ("server-url", "Server URL", - "URL to which events are uploaded", - NULL, - G_PARAM_CONSTRUCT | G_PARAM_WRITABLE | - G_PARAM_STATIC_STRINGS); - /* * EmerDaemon:network-send-interval: * @@ -1703,10 +1661,6 @@ emer_daemon_new (const gchar *persistent_cache_directory, * emer_daemon_new_full: * @rand: (allow-none): random number generator to use for randomized * exponential backoff, or %NULL to use the default. - * @server_url: (allow-none): the URL (including protocol and, optionally, port - * number) to which to upload events, or %NULL to use the default. Must - * include trailing forward slash. If the port number is unspecified, it - * defaults to 443 (the standard port used by SSL). * @network_send_interval: frequency in seconds with which the client will * attempt a network send request. * @permissions_provider: The #EmerPermissionsProvider to supply information @@ -1726,7 +1680,6 @@ emer_daemon_new (const gchar *persistent_cache_directory, */ EmerDaemon * emer_daemon_new_full (GRand *rand, - const gchar *server_url, guint network_send_interval, EmerPermissionsProvider *permissions_provider, EmerPersistentCache *persistent_cache, @@ -1735,7 +1688,6 @@ emer_daemon_new_full (GRand *rand, { return g_object_new (EMER_TYPE_DAEMON, "random-number-generator", rand, - "server-url", server_url, "network-send-interval", network_send_interval, "permissions-provider", permissions_provider, "persistent-cache", persistent_cache, @@ -1837,11 +1789,7 @@ emer_daemon_upload_events (EmerDaemon *self, GAsyncReadyCallback callback, gpointer user_data) { - - gchar *environment = - emer_permissions_provider_get_environment (self->permissions_provider); - upload_events (self, G_MAXSIZE, environment, callback, user_data); - g_free (environment); + upload_events (self, G_MAXSIZE, callback, user_data); } /* emer_daemon_upload_events_finish: diff --git a/daemon/emer-daemon.h b/daemon/emer-daemon.h index 92c4d87..ba4896b 100644 --- a/daemon/emer-daemon.h +++ b/daemon/emer-daemon.h @@ -40,7 +40,6 @@ EmerDaemon * emer_daemon_new (const gchar EmerPermissionsProvider *permissions_provider); EmerDaemon * emer_daemon_new_full (GRand *rand, - const gchar *server_url, guint network_send_interval, EmerPermissionsProvider *permissions_provider, EmerPersistentCache *persistent_cache, diff --git a/daemon/emer-permissions-provider.c b/daemon/emer-permissions-provider.c index 9b6019a..fc277b3 100644 --- a/daemon/emer-permissions-provider.c +++ b/daemon/emer-permissions-provider.c @@ -31,8 +31,10 @@ #include "shared/metrics-util.h" -typedef struct _EmerPermissionsProviderPrivate +typedef struct _EmerPermissionsProvider { + GObject parent; + /* Permissions, cached from config file */ GKeyFile *permissions; @@ -44,9 +46,9 @@ typedef struct _EmerPermissionsProviderPrivate /* For reading the OSTree config file */ GFile *ostree_config_file; -} EmerPermissionsProviderPrivate; +} EmerPermissionsProvider; -G_DEFINE_TYPE_WITH_PRIVATE (EmerPermissionsProvider, emer_permissions_provider, G_TYPE_OBJECT) +G_DEFINE_TYPE (EmerPermissionsProvider, emer_permissions_provider, G_TYPE_OBJECT) #define DAEMON_GLOBAL_GROUP_NAME "global" #define DAEMON_ENABLED_KEY_NAME "enabled" @@ -58,7 +60,8 @@ G_DEFINE_TYPE_WITH_PRIVATE (EmerPermissionsProvider, emer_permissions_provider, "[" DAEMON_GLOBAL_GROUP_NAME "]\n" \ DAEMON_ENABLED_KEY_NAME "=true\n" \ DAEMON_UPLOADING_ENABLED_KEY_NAME "=false\n" \ - DAEMON_ENVIRONMENT_KEY_NAME "=production\n" + DAEMON_ENVIRONMENT_KEY_NAME "=production\n" \ + DAEMON_SERVER_URL_KEY_NAME "=" DEFAULT_METRICS_SERVER_URL "\n" enum { @@ -77,10 +80,7 @@ static GParamSpec *emer_permissions_provider_props[NPROPS] = { NULL, }; static void load_fallback_data (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - if (g_key_file_load_from_data (priv->permissions, FALLBACK_CONFIG_FILE_DATA, + if (g_key_file_load_from_data (self->permissions, FALLBACK_CONFIG_FILE_DATA, -1, G_KEY_FILE_NONE, NULL)) return; @@ -93,33 +93,25 @@ load_fallback_data (EmerPermissionsProvider *self) static void write_config_file_sync (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - gchar *permissions_config_file_path = - g_file_get_path (priv->permissions_config_file); - GError *error = NULL; + g_autofree gchar *permissions_config_file_path = + g_file_get_path (self->permissions_config_file); + g_autoptr(GError) error = NULL; - if (!g_key_file_save_to_file (priv->permissions, permissions_config_file_path, + if (!g_key_file_save_to_file (self->permissions, permissions_config_file_path, &error)) { g_critical ("Could not write to permissions config file '%s'. Error: %s.", permissions_config_file_path, error->message); - g_clear_error (&error); } - - g_free (permissions_config_file_path); } static gboolean write_config_file_idle_cb (gpointer data) { EmerPermissionsProvider *self = EMER_PERMISSIONS_PROVIDER (data); - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); write_config_file_sync (self); - priv->write_config_file_idle_id = 0; + self->write_config_file_idle_id = 0; return G_SOURCE_REMOVE; } @@ -128,13 +120,10 @@ write_config_file_idle_cb (gpointer data) static void schedule_config_file_update (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - if (priv->write_config_file_idle_id != 0) + if (self->write_config_file_idle_id != 0) return; - priv->write_config_file_idle_id = + self->write_config_file_idle_id = g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, write_config_file_idle_cb, g_object_ref (self), @@ -146,14 +135,11 @@ default. Also emits a property notification. */ static void read_config_file_sync (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); + g_autoptr(GError) error = NULL; - GError *error = NULL; - - gchar *path = g_file_get_path (priv->permissions_config_file); + const gchar *path = g_file_peek_path (self->permissions_config_file); gboolean load_succeeded = - g_key_file_load_from_file (priv->permissions, path, G_KEY_FILE_NONE, &error); + g_key_file_load_from_file (self->permissions, path, G_KEY_FILE_NONE, &error); if (!load_succeeded) { load_fallback_data (self); @@ -169,8 +155,6 @@ read_config_file_sync (EmerPermissionsProvider *self) g_clear_error (&error); } - g_free (path); - GParamSpec *daemon_enabled_pspec = emer_permissions_provider_props[PROP_DAEMON_ENABLED]; g_object_notify_by_pspec (G_OBJECT (self), daemon_enabled_pspec); @@ -187,10 +171,7 @@ read_config_file_sync (EmerPermissionsProvider *self) static gchar * get_ostree_url_from_file (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - gchar *path = g_file_get_path (priv->ostree_config_file); + const gchar *path = g_file_peek_path (self->ostree_config_file); if (path == NULL) { @@ -198,31 +179,24 @@ get_ostree_url_from_file (EmerPermissionsProvider *self) return NULL; } - GKeyFile *ostree_configuration_key_file = g_key_file_new (); - GError *error = NULL; + g_autoptr(GKeyFile) ostree_configuration_key_file = g_key_file_new (); + g_autoptr(GError) error = NULL; if (!g_key_file_load_from_file (ostree_configuration_key_file, path, G_KEY_FILE_NONE, &error)) { g_warning ("Unable to load OSTree GKeyFile from given OSTree config " "file path %s. Error: %s.", path, error->message); - g_free (path); - g_clear_error (&error); return NULL; } - g_free (path); - gchar *ostree_url = g_key_file_get_value (ostree_configuration_key_file, "remote \"eos\"", "url", &error); - g_key_file_unref (ostree_configuration_key_file); - if (ostree_url == NULL) { g_warning ("Unable to read OSTree URL from given OSTree config file. " "Error: %s.", error->message); - g_clear_error (&error); } return ostree_url; @@ -232,15 +206,14 @@ static gchar * get_ostree_url_from_ostree_repo (void) { OstreeRepo *ostree_repo = ostree_repo_new_default (); + g_autoptr(GError) error = NULL; - GError *error = NULL; if (!ostree_repo_open (ostree_repo, NULL /* GCancellable */, &error)) { /* Don't warn if simply not on an OSTree system. */ if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) g_warning ("Unable to open OSTree repo: %s", error->message); - g_clear_error (&error); g_object_unref (ostree_repo); return NULL; } @@ -263,7 +236,6 @@ get_ostree_url_from_ostree_repo (void) if (ostree_url == NULL) { g_warning ("Unable to read OSTree URL. Error: %s.", error->message); - g_clear_error (&error); } return ostree_url; @@ -272,10 +244,7 @@ get_ostree_url_from_ostree_repo (void) static gchar * read_ostree_url (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - if (priv->ostree_config_file == NULL) + if (self->ostree_config_file == NULL) return get_ostree_url_from_ostree_repo (); else return get_ostree_url_from_file (self); @@ -284,11 +253,8 @@ read_ostree_url (EmerPermissionsProvider *self) static gchar * read_environment (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - GError *error = NULL; - gchar *environment = g_key_file_get_value (priv->permissions, + g_autoptr(GError) error = NULL; + gchar *environment = g_key_file_get_value (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_ENVIRONMENT_KEY_NAME, &error); @@ -298,7 +264,6 @@ read_environment (EmerPermissionsProvider *self) "Returning default value. Error: %s.", DAEMON_GLOBAL_GROUP_NAME, DAEMON_ENVIRONMENT_KEY_NAME, error->message); - g_error_free (error); } if (g_strcmp0 (environment, "dev") != 0 && @@ -325,10 +290,7 @@ static void set_environment (EmerPermissionsProvider *self, const gchar *environment) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - g_key_file_set_string (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + g_key_file_set_string (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_ENVIRONMENT_KEY_NAME, environment); schedule_config_file_update (self); @@ -350,10 +312,7 @@ static void set_config_file_path (EmerPermissionsProvider *self, const gchar *path) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - priv->permissions_config_file = g_file_new_for_path (path); + self->permissions_config_file = g_file_new_for_path (path); } /* Construct-only setter */ @@ -364,10 +323,7 @@ set_ostree_config_file_path (EmerPermissionsProvider *self, if (path == NULL) return; - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - priv->ostree_config_file = g_file_new_for_path (path); + self->ostree_config_file = g_file_new_for_path (path); } static void @@ -428,11 +384,6 @@ emer_permissions_provider_set_property (GObject *object, g_value_get_boolean (value)); break; - case PROP_SERVER_URL: - emer_permissions_provider_set_server_url (self, - g_value_get_string (value)); - break; - default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); } @@ -442,12 +393,10 @@ static void emer_permissions_provider_finalize (GObject *object) { EmerPermissionsProvider *self = EMER_PERMISSIONS_PROVIDER (object); - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - g_key_file_unref (priv->permissions); - g_clear_object (&priv->permissions_config_file); - g_clear_object (&priv->ostree_config_file); + g_key_file_unref (self->permissions); + g_clear_object (&self->permissions_config_file); + g_clear_object (&self->ostree_config_file); G_OBJECT_CLASS (emer_permissions_provider_parent_class)->finalize (object); } @@ -486,7 +435,7 @@ emer_permissions_provider_class_init (EmerPermissionsProviderClass *klass) g_param_spec_string ("server-url", "Server URL", "Metrics server URL to use", NULL, - G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, NPROPS, emer_permissions_provider_props); @@ -495,11 +444,8 @@ emer_permissions_provider_class_init (EmerPermissionsProviderClass *klass) static void emer_permissions_provider_init (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - priv->permissions = g_key_file_new (); - priv->ostree_config_file = NULL; + self->permissions = g_key_file_new (); + self->ostree_config_file = NULL; } /* PUBLIC API */ @@ -551,12 +497,9 @@ emer_permissions_provider_new_full (const gchar *permissions_config_file_path, gboolean emer_permissions_provider_get_daemon_enabled (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - GError *error = NULL; + g_autoptr(GError) error = NULL; gboolean daemon_enabled = - g_key_file_get_boolean (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + g_key_file_get_boolean (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_ENABLED_KEY_NAME, &error); if (error != NULL) { @@ -564,7 +507,6 @@ emer_permissions_provider_get_daemon_enabled (EmerPermissionsProvider *self) "Returning default value. Error: %s.", DAEMON_GLOBAL_GROUP_NAME, DAEMON_ENABLED_KEY_NAME, error->message); - g_error_free (error); } return daemon_enabled; @@ -581,10 +523,7 @@ void emer_permissions_provider_set_daemon_enabled (EmerPermissionsProvider *self, gboolean enabled) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - g_key_file_set_boolean (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + g_key_file_set_boolean (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_ENABLED_KEY_NAME, enabled); schedule_config_file_update (self); @@ -608,12 +547,9 @@ emer_permissions_provider_set_daemon_enabled (EmerPermissionsProvider *self, gboolean emer_permissions_provider_get_uploading_enabled (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - GError *error = NULL; + g_autoptr(GError) error = NULL; gboolean uploading_enabled = - g_key_file_get_boolean (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + g_key_file_get_boolean (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_UPLOADING_ENABLED_KEY_NAME, &error); if (error != NULL) { @@ -621,7 +557,6 @@ emer_permissions_provider_get_uploading_enabled (EmerPermissionsProvider *self) "Returning default value. Error: %s.", DAEMON_GLOBAL_GROUP_NAME, DAEMON_UPLOADING_ENABLED_KEY_NAME, error->message); - g_error_free (error); return TRUE; } @@ -641,10 +576,7 @@ void emer_permissions_provider_set_uploading_enabled (EmerPermissionsProvider *self, gboolean enabled) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - g_key_file_set_boolean (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + g_key_file_set_boolean (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_UPLOADING_ENABLED_KEY_NAME, enabled); schedule_config_file_update (self); @@ -671,7 +603,7 @@ emer_permissions_provider_get_environment (EmerPermissionsProvider *self) read_config_file_sync (self); gchar *environment = read_environment (self); - gchar *ostree_url = read_ostree_url (self); + g_autofree gchar *ostree_url = read_ostree_url (self); if (ostree_url == NULL) { @@ -691,7 +623,6 @@ emer_permissions_provider_get_environment (EmerPermissionsProvider *self) set_environment (self, environment); } - g_clear_pointer (&ostree_url, g_free); return environment; } @@ -699,53 +630,35 @@ emer_permissions_provider_get_environment (EmerPermissionsProvider *self) * emer_permissions_provider_get_server_url: * @self: the permissions provider * - * Gets the metrics server URL from the configuration file. + * Gets the metrics server base URL from the configuration file, with any + * placeholders expanded. * * Returns: The metrics server URL. */ gchar * emer_permissions_provider_get_server_url (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); + g_autofree gchar *server_url = NULL; + g_autoptr(GError) error = NULL; - GError *error = NULL; - gchar *server_url = - g_key_file_get_value (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + server_url = + g_key_file_get_value (self->permissions, DAEMON_GLOBAL_GROUP_NAME, DAEMON_SERVER_URL_KEY_NAME, &error); - if (error != NULL) + if (server_url == NULL) { - g_debug ("Couldn't find key '%s:%s' in permissions config file. " - "Returning default value. Error: %s.", - DAEMON_GLOBAL_GROUP_NAME, DAEMON_SERVER_URL_KEY_NAME, - error->message); - g_error_free (error); - return NULL; + g_debug ("%s: %s", G_STRFUNC, error->message); + server_url = g_strdup (DEFAULT_METRICS_SERVER_URL); } - return server_url; -} - -/* - * emer_permissions_provider_set_server_url: - * @self: the permissions provider - * @server_url: the metrics server URL to use - * - * Sets the metrics server URL in the configuration file. - */ -void -emer_permissions_provider_set_server_url (EmerPermissionsProvider *self, - const gchar *server_url) -{ - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - g_key_file_set_value (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, - DAEMON_SERVER_URL_KEY_NAME, server_url); + if (strstr (server_url, "${environment}") != NULL) + { + GString *s = g_string_new (server_url); + g_autofree gchar *environment = + emer_permissions_provider_get_environment (self); + g_string_replace (s, "${environment}", environment, 0); - schedule_config_file_update (self); + return g_string_free (s, FALSE); + } - GParamSpec *server_url_pspec = - emer_permissions_provider_props[PROP_SERVER_URL]; - g_object_notify_by_pspec (G_OBJECT (self), server_url_pspec); + return g_steal_pointer (&server_url); } diff --git a/daemon/emer-permissions-provider.h b/daemon/emer-permissions-provider.h index e2bb52f..36ef331 100644 --- a/daemon/emer-permissions-provider.h +++ b/daemon/emer-permissions-provider.h @@ -28,43 +28,11 @@ G_BEGIN_DECLS #define EMER_TYPE_PERMISSIONS_PROVIDER emer_permissions_provider_get_type() - -#define EMER_PERMISSIONS_PROVIDER(obj) \ - (G_TYPE_CHECK_INSTANCE_CAST ((obj), \ - EMER_TYPE_PERMISSIONS_PROVIDER, EmerPermissionsProvider)) - -#define EMER_PERMISSIONS_PROVIDER_CLASS(klass) \ - (G_TYPE_CHECK_CLASS_CAST ((klass), \ - EMER_TYPE_PERMISSIONS_PROVIDER, EmerPermissionsProviderClass)) - -#define EMER_IS_PERMISSIONS_PROVIDER(obj) \ - (G_TYPE_CHECK_INSTANCE_TYPE ((obj), \ - EMER_TYPE_PERMISSIONS_PROVIDER)) - -#define EMER_IS_PERMISSIONS_PROVIDER_CLASS(klass) \ - (G_TYPE_CHECK_CLASS_TYPE ((klass), \ - EMER_TYPE_PERMISSIONS_PROVIDER)) - -#define EMER_PERMISSIONS_PROVIDER_GET_CLASS(obj) \ - (G_TYPE_INSTANCE_GET_CLASS ((obj), \ - EMER_TYPE_PERMISSIONS_PROVIDER, EmerPermissionsProviderClass)) - -typedef struct _EmerPermissionsProvider EmerPermissionsProvider; -typedef struct _EmerPermissionsProviderClass EmerPermissionsProviderClass; - -struct _EmerPermissionsProvider -{ - GObject parent; -}; - -struct _EmerPermissionsProviderClass -{ - GObjectClass parent_class; -}; - -G_DEFINE_AUTOPTR_CLEANUP_FUNC(EmerPermissionsProvider, g_object_unref) - -GType emer_permissions_provider_get_type (void) G_GNUC_CONST; +G_DECLARE_FINAL_TYPE (EmerPermissionsProvider, + emer_permissions_provider, + EMER, + PERMISSIONS_PROVIDER, + GObject) EmerPermissionsProvider *emer_permissions_provider_new (void); @@ -85,9 +53,6 @@ gchar *emer_permissions_provider_get_environment (EmerPe gchar *emer_permissions_provider_get_server_url (EmerPermissionsProvider *self); -void emer_permissions_provider_set_server_url (EmerPermissionsProvider *self, - const gchar *server_url); - G_END_DECLS #endif /* EMER_PERMISSIONS_PROVIDER_H */ diff --git a/debian/control b/debian/control index c7f6e71..92bcb4d 100644 --- a/debian/control +++ b/debian/control @@ -4,11 +4,11 @@ Maintainer: Endless OS Team Build-Depends: ca-certificates, debhelper-compat (= 13), eos-metrics-0-dev (>= 0.2), - libglib2.0-dev (>= 2.68), + libglib2.0-dev (>= 2.74), libostree-dev (>= 2013.7), libpolkit-gobject-1-dev, libsoup-3.0-dev, - meson (>= 0.52), + meson (>= 0.55), pkg-config, python3-dbus, python3-dbusmock, diff --git a/debian/rules b/debian/rules index 1eb7881..6331803 100755 --- a/debian/rules +++ b/debian/rules @@ -14,7 +14,7 @@ # Build our documentation as well override_dh_auto_configure: - dh_auto_configure -- -Ddefault_metrics_server_url='https://${environment}.metrics.endlessm.com' + dh_auto_configure -- -Ddefault_metrics_server_url='https://$${environment}.metrics.endlessm.com' # Avoid changing the owner of configuration files and the persistent cache # directory to root:root. diff --git a/meson.build b/meson.build index 52929d9..7796de1 100644 --- a/meson.build +++ b/meson.build @@ -3,19 +3,24 @@ project('eos-event-recorder-daemon', license: 'GPL-2.0-or-later', version: '0.0.0', default_options: ['c_std=c11', 'warning_level=2'], - meson_version: '>= 0.52.0', + meson_version: '>= 0.55', ) cc = meson.get_compiler('c') -glib_dep = dependency('glib-2.0', version: '>= 2.68') +glib_major_version = '2' +glib_minor_version = '74' +glib_dep_version = '>= @0@.@1@'.format(glib_major_version, glib_minor_version) +glib_version_define = 'GLIB_VERSION_@0@_@1@'.format(glib_major_version, glib_minor_version) + +glib_dep = dependency('glib-2.0', version: glib_dep_version) eosmetrics_dep = dependency('eosmetrics-0', version: '>= 0.2') polkit_gobject_dep = dependency('polkit-gobject-1') emer_shared_required_modules = [ glib_dep, - dependency('gio-unix-2.0', version: '>= 2.46'), - dependency('gobject-2.0', version: '>= 2.58'), + dependency('gio-unix-2.0', version: glib_dep_version), + dependency('gobject-2.0', version: glib_dep_version), dependency('sqlite3'), dependency('uuid'), ] @@ -53,8 +58,8 @@ add_project_arguments( '-DPERMISSIONS_FILE="@0@"'.format(permissions_file), '-DPERSISTENT_CACHE_DIR="@0@"'.format(persistent_cache_dir), '-DSYSCONFDIR="@0@"'.format(get_option('sysconfdir')), - '-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_68', - '-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_68', + '-DGLIB_VERSION_MIN_REQUIRED=@0@'.format(glib_version_define), + '-DGLIB_VERSION_MAX_ALLOWED=@0@'.format(glib_version_define), '-Wno-unused-parameter', ], language: 'c', @@ -89,10 +94,7 @@ if valgrind.found() # --trace-children=yes because that test also spawns other things such as # a dbus-daemon and we don't want to valgrind those. env: { - 'EXE_WRAPPER': ' '.join( - # TODO: use .full_path() once we require meson ≥ 0.55 - [valgrind.path()] + valgrind_arguments - ), + 'EXE_WRAPPER': ' '.join([valgrind.full_path()] + valgrind_arguments), }, timeout_multiplier: 10, ) diff --git a/tests/daemon/mock-permissions-provider.c b/tests/daemon/mock-permissions-provider.c index 5d1c110..f2c94d6 100644 --- a/tests/daemon/mock-permissions-provider.c +++ b/tests/daemon/mock-permissions-provider.c @@ -27,27 +27,43 @@ #include #include -typedef struct _EmerPermissionsProviderPrivate +typedef struct _EmerPermissionsProvider { + GObject parent; + gboolean daemon_enabled; gboolean uploading_enabled; -} EmerPermissionsProviderPrivate; -G_DEFINE_TYPE_WITH_PRIVATE (EmerPermissionsProvider, emer_permissions_provider, G_TYPE_OBJECT) + gchar *server_url; +} EmerPermissionsProvider; + +G_DEFINE_TYPE (EmerPermissionsProvider, emer_permissions_provider, G_TYPE_OBJECT) + +static void +emer_permissions_provider_finalize (GObject *object) +{ + EmerPermissionsProvider *self = EMER_PERMISSIONS_PROVIDER (object); + + g_clear_pointer (&self->server_url, g_free); + + G_OBJECT_CLASS (emer_permissions_provider_parent_class)->finalize (object); +} static void emer_permissions_provider_class_init (EmerPermissionsProviderClass *klass) { + GObjectClass *object_class = G_OBJECT_CLASS (klass); + + object_class->finalize = emer_permissions_provider_finalize; + } static void emer_permissions_provider_init (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - priv->daemon_enabled = TRUE; - priv->uploading_enabled = TRUE; + self->daemon_enabled = TRUE; + self->uploading_enabled = TRUE; + self->server_url = NULL; } /* MOCK PUBLIC API */ @@ -65,23 +81,25 @@ emer_permissions_provider_new_full (const gchar *config_file_path, return emer_permissions_provider_new (); } +EmerPermissionsProvider * +mock_permissions_provider_new (const gchar *server_url) +{ + EmerPermissionsProvider *self = emer_permissions_provider_new (); + self->server_url = g_strdup (server_url); + return self; +} + gboolean emer_permissions_provider_get_daemon_enabled (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - return priv->daemon_enabled; + return self->daemon_enabled; } void emer_permissions_provider_set_daemon_enabled (EmerPermissionsProvider *self, gboolean enabled) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - priv->daemon_enabled = enabled; + self->daemon_enabled = enabled; /* Emit a property notification even though there isn't a property by this * name in this mock object. @@ -92,10 +110,7 @@ emer_permissions_provider_set_daemon_enabled (EmerPermissionsProvider *self, gboolean emer_permissions_provider_get_uploading_enabled (EmerPermissionsProvider *self) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - return priv->uploading_enabled; + return self->uploading_enabled; } gchar * @@ -116,7 +131,7 @@ emer_permissions_provider_get_environment (EmerPermissionsProvider *self) gchar * emer_permissions_provider_get_server_url (EmerPermissionsProvider *self) { - return NULL; + return g_strdup (self->server_url); } /* API OF MOCK OBJECT */ @@ -127,10 +142,7 @@ void emer_permissions_provider_set_uploading_enabled (EmerPermissionsProvider *self, gboolean uploading_enabled) { - EmerPermissionsProviderPrivate *priv = - emer_permissions_provider_get_instance_private (self); - - priv->uploading_enabled = uploading_enabled; + self->uploading_enabled = uploading_enabled; /* Emit a property notification even though there isn't a property by this * name in this mock object. diff --git a/tests/daemon/mock-permissions-provider.h b/tests/daemon/mock-permissions-provider.h index 1aec02e..8f9eb9f 100644 --- a/tests/daemon/mock-permissions-provider.h +++ b/tests/daemon/mock-permissions-provider.h @@ -29,6 +29,8 @@ G_BEGIN_DECLS +EmerPermissionsProvider *mock_permissions_provider_new (const gchar *server_url); + G_END_DECLS #endif /* MOCK_PERMISSIONS_PROVIDER_H */ diff --git a/tests/daemon/test-daemon.c b/tests/daemon/test-daemon.c index 819863c..f48f22c 100644 --- a/tests/daemon/test-daemon.c +++ b/tests/daemon/test-daemon.c @@ -584,22 +584,18 @@ get_events_from_request (GByteArray *request, emtr_util_get_current_time (CLOCK_REALTIME, &curr_absolute_time); g_assert_true (get_succeeded); - GBytes *request_bytes = g_bytes_new (request->data, request->len); + g_autoptr(GBytes) request_bytes = g_bytes_new (request->data, request->len); - gchar *checksum = + g_autofree gchar *checksum = g_compute_checksum_for_bytes (G_CHECKSUM_SHA512, request_bytes); - gchar *expected_request_path = g_build_filename ("/3/", checksum, NULL); - g_free (checksum); + g_autofree gchar *expected_request_path = g_build_filename ("/3/", checksum, NULL); g_assert_cmpstr (fixture->request_path, ==, expected_request_path); - g_free (expected_request_path); const GVariantType *REQUEST_FORMAT = G_VARIANT_TYPE ("(xxsa{ss}ya(aysxmv)a(ayssumv))"); GVariant *request_variant = g_variant_new_from_bytes (REQUEST_FORMAT, request_bytes, FALSE); - g_bytes_unref (request_bytes); - g_assert_true (g_variant_is_normal_form (request_variant)); g_variant_ref_sink (request_variant); @@ -826,7 +822,6 @@ create_test_object (Fixture *fixture) { fixture->test_object = emer_daemon_new_full (g_rand_new_with_seed (18), - fixture->server_url, 2 /* network send interval */, fixture->mock_permissions_provider, fixture->mock_persistent_cache, @@ -854,7 +849,7 @@ setup_most (Fixture *fixture, fixture->server_url = get_server_url (fixture->mock_server); - fixture->mock_permissions_provider = emer_permissions_provider_new (); + fixture->mock_permissions_provider = mock_permissions_provider_new (fixture->server_url); fixture->mock_persistent_cache = NULL; /* Not actually a mock! */ fixture->mock_aggregate_tally = emer_aggregate_tally_new (g_get_user_cache_dir ()); diff --git a/tests/daemon/test-permissions-provider.c b/tests/daemon/test-permissions-provider.c index e5b44c2..aa00ba0 100644 --- a/tests/daemon/test-permissions-provider.c +++ b/tests/daemon/test-permissions-provider.c @@ -28,52 +28,65 @@ #include #include -#define PERMISSIONS_CONFIG_FILE_ENABLED_TEST \ - "[global]\n" \ - "enabled=true\n" \ - "uploading_enabled=true\n" \ - "environment=test" -#define PERMISSIONS_CONFIG_FILE_DISABLED_TEST \ - "[global]\n" \ - "enabled=false\n" \ - "uploading_enabled=true\n" \ - "environment=test" -#define PERMISSIONS_CONFIG_FILE_UPLOADING_DISABLED_TEST \ - "[global]\n" \ - "enabled=true\n" \ - "uploading_enabled=false\n" \ - "environment=test" -#define PERMISSIONS_CONFIG_FILE_INVALID \ - "lavubeu;f'w943ty[jdn;fbl\n" -#define PERMISSIONS_CONFIG_FILE_ENABLED_DEV \ - "[global]\n" \ - "enabled=true\n" \ - "uploading_enabled=true\n" \ - "environment=dev" -#define PERMISSIONS_CONFIG_FILE_ENABLED_PRODUCTION \ - "[global]\n" \ - "enabled=true\n" \ - "uploading_enabled=true\n" \ - "environment=production" -#define PERMISSIONS_CONFIG_FILE_ENABLED_INVALID_ENVIRONMENT \ - "[global]\n" \ - "enabled=true\n" \ - "uploading_enabled=true\n" \ - "environment=invalid" -#define OSTREE_CONFIG_FILE_STAGING_URL \ - "[core]\n" \ - "repo_version=1\n" \ - "mode=bare\n\n" \ - "[remote \"eos\"]\n" \ - "url=http://fakeurl.with/staging/in/path\n" \ - "branches=master/i386;" -#define OSTREE_CONFIG_FILE_NON_STAGING_URL \ - "[core]\n" \ - "repo_version=1\n" \ - "mode=bare\n\n" \ - "[remote \"eos\"]\n" \ - "url=http://fakeurl.without/term/in/path\n" \ - "branches=master/i386;" +const char *PERMISSIONS_CONFIG_FILE_ENABLED_TEST = + "[global]\n" + "enabled=true\n" + "uploading_enabled=true\n" + "environment=test"; +const char *PERMISSIONS_CONFIG_FILE_DISABLED_TEST = + "[global]\n" + "enabled=false\n" + "uploading_enabled=true\n" + "environment=test"; +const char *PERMISSIONS_CONFIG_FILE_UPLOADING_DISABLED_TEST = + "[global]\n" + "enabled=true\n" + "uploading_enabled=false\n" + "environment=test"; +const char *PERMISSIONS_CONFIG_FILE_INVALID = + "lavubeu;f'w943ty[jdn;fbl\n"; +const char *PERMISSIONS_CONFIG_FILE_ENABLED_DEV = + "[global]\n" + "enabled=true\n" + "uploading_enabled=true\n" + "environment=dev"; +const char *PERMISSIONS_CONFIG_FILE_ENABLED_PRODUCTION = + "[global]\n" + "enabled=true\n" + "uploading_enabled=true\n" + "environment=production"; +const char *PERMISSIONS_CONFIG_FILE_ENABLED_INVALID_ENVIRONMENT = + "[global]\n" + "enabled=true\n" + "uploading_enabled=true\n" + "environment=invalid"; +const char *PERMISSIONS_CONFIG_FILE_WITH_TEMPLATE_URL = + "[global]\n" + "enabled=true\n" + "uploading_enabled=true\n" + "environment=dev\n" + "server_url=https://${environment}.example.com/"; +const char *PERMISSIONS_CONFIG_FILE_WITH_PLAIN_URL = + "[global]\n" + "enabled=true\n" + "uploading_enabled=true\n" + "environment=dev\n" + "server_url=https://example.com/"; + +const char *OSTREE_CONFIG_FILE_STAGING_URL = + "[core]\n" + "repo_version=1\n" + "mode=bare\n\n" + "[remote \"eos\"]\n" + "url=http://fakeurl.with/staging/in/path\n" + "branches=master/i386;"; +const char *OSTREE_CONFIG_FILE_NON_STAGING_URL = + "[core]\n" + "repo_version=1\n" + "mode=bare\n\n" + "[remote \"eos\"]\n" + "url=http://fakeurl.without/term/in/path\n" + "branches=master/i386;"; typedef struct { GFile *permissions_config_file; @@ -520,6 +533,46 @@ test_permissions_provider_set_uploading_enabled_updates_config_file (Fixture "^uploading_enabled=false$")); } +static void +test_permissions_provider_returns_valid_default_url (Fixture *fixture, + gconstpointer unused) +{ + g_autofree gchar *url = NULL; + g_autoptr(GUri) gellar = NULL; + g_autoptr(GError) error = NULL; + + url = emer_permissions_provider_get_server_url (fixture->test_object); + g_assert_nonnull (url); + + /* We can't make any specific assertion about the URL because it is a + * build-time parameter. But we can verify that that build-time parameter + * is a valid URL. + */ + gellar = g_uri_parse (url, G_URI_FLAGS_NONE, &error); + g_assert_no_error (error); + +} + +static void +test_permissions_provider_expands_url (Fixture *fixture, + gconstpointer unused) +{ + g_autofree gchar *url = NULL; + + url = emer_permissions_provider_get_server_url (fixture->test_object); + g_assert_cmpstr (url, ==, "https://dev.example.com/"); +} + +static void +test_permissions_provider_handles_plain_url (Fixture *fixture, + gconstpointer unused) +{ + g_autofree gchar *url = NULL; + + url = emer_permissions_provider_get_server_url (fixture->test_object); + g_assert_cmpstr (url, ==, "https://example.com/"); +} + gint main (gint argc, const gchar * const argv[]) @@ -608,6 +661,18 @@ main (gint argc, PERMISSIONS_CONFIG_FILE_ENABLED_TEST, setup_with_config_file, test_permissions_provider_set_uploading_enabled_updates_config_file); + ADD_PERMISSIONS_PROVIDER_TEST ("/permissions-provider/get-server-url/returns-valid-default-url", + PERMISSIONS_CONFIG_FILE_ENABLED_PRODUCTION, + setup_with_config_file, + test_permissions_provider_returns_valid_default_url); + ADD_PERMISSIONS_PROVIDER_TEST ("/permissions-provider/get-server-url/expands-template", + PERMISSIONS_CONFIG_FILE_WITH_TEMPLATE_URL, + setup_with_config_file, + test_permissions_provider_expands_url); + ADD_PERMISSIONS_PROVIDER_TEST ("/permissions-provider/get-server-url/handles-plain-url", + PERMISSIONS_CONFIG_FILE_WITH_PLAIN_URL, + setup_with_config_file, + test_permissions_provider_handles_plain_url); #undef ADD_PERMISSIONS_PROVIDER_TEST diff --git a/tests/test-opt-out-integration.py b/tests/test-opt-out-integration.py index 0736ccc..45b43c8 100755 --- a/tests/test-opt-out-integration.py +++ b/tests/test-opt-out-integration.py @@ -156,17 +156,15 @@ def test_upload_doesnt_change_config(self): EmerPermissionsProvider:uploading-enabled so caused that property to be set to TRUE. """ - # Check defaults look good and erase the file before our next change - self._check_config_file(enabled="true", uploading_enabled="false") - - with self.assertRaisesRegex( - dbus.exceptions.DBusException, r"uploading is disabled" - ) as context: - self.interface.UploadEvents() - self.assertEqual( - context.exception.get_dbus_name(), - "com.endlessm.Metrics.Error.UploadingDisabled", - ) + for _ in range(2): + with self.assertRaisesRegex( + dbus.exceptions.DBusException, r"uploading is disabled" + ) as context: + self.interface.UploadEvents() + self.assertEqual( + context.exception.get_dbus_name(), + "com.endlessm.Metrics.Error.UploadingDisabled", + ) self._check_config_file(enabled="true", uploading_enabled="false") @@ -262,7 +260,7 @@ def _check_config_file(self, enabled, uploading_enabled): self.assertEqual(config.get("global", "uploading_enabled"), uploading_enabled) # erase the file after reading it to guarantee that the next time it - # exists, it's up to date. the daemon doesn't read it once started. + # exists, it's up to date. os.unlink(self.config_file)