Bug 1679793: Change the RLB Configuration to use PathBuf for the data…

CHANGELOG.md

@@ -4,7 +4,8 @@
 
 * Kotlin
   * The `testGetValue` APIs now include a message on the `NullPointerException` thrown when the value is missing.
-
+* RLB
+  * **Breaking change:** `Configuration.data_path` is now a `std::path::PathBuf`([#1493](https://github.com/mozilla/glean/pull/1493)). 
 # v34.1.0 (2021-02-04)
 
 [Full changelog](https://github.com/mozilla/glean/compare/v34.0.0...v34.1.0)

glean-core/rlb/examples/prototype.rs

@@ -3,6 +3,7 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use std::env;
+use std::path::PathBuf;
 
 use once_cell::sync::Lazy;
 use tempfile::Builder;
@@ -36,10 +37,10 @@ fn main() {
     let mut args = env::args().skip(1);
 
     let data_path = if let Some(path) = args.next() {
-        path
+        PathBuf::from(path)
     } else {
         let root = Builder::new().prefix("simple-db").tempdir().unwrap();
-        root.path().display().to_string()
+        root.path().to_path_buf()
     };
 
     let cfg = Configuration {

glean-core/rlb/src/common_test.rs

@@ -36,7 +36,7 @@ pub(crate) fn new_glean(
     clear_stores: bool,
 ) -> tempfile::TempDir {
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = match configuration {
         Some(c) => c,

glean-core/rlb/src/configuration.rs

@@ -4,6 +4,8 @@
 
 use crate::net::PingUploader;
 
+use std::path::PathBuf;
+
 /// The default server pings are sent to.
 pub(crate) const DEFAULT_GLEAN_ENDPOINT: &str = "https://incoming.telemetry.mozilla.org";
 
@@ -15,7 +17,7 @@ pub struct Configuration {
     /// Whether upload should be enabled.
     pub upload_enabled: bool,
     /// Path to a directory to store all data in.
-    pub data_path: String,
+    pub data_path: PathBuf,
     /// The application ID (will be sanitized during initialization).
     pub application_id: String,
     /// The maximum number of events to store before sending a ping containing events.

glean-core/rlb/src/lib.rs

@@ -171,7 +171,7 @@ pub fn initialize(cfg: Configuration, client_info: ClientInfoMetrics) {
         .spawn(move || {
             let core_cfg = glean_core::Configuration {
                 upload_enabled: cfg.upload_enabled,
-                data_path: cfg.data_path.clone(),
+                data_path: cfg.data_path.into_os_string().into_string().unwrap(),
                 application_id: cfg.application_id.clone(),
                 language_binding_name: LANGUAGE_BINDING_NAME.into(),
                 max_events: cfg.max_events,

glean-core/rlb/src/test.rs

@@ -4,7 +4,6 @@
 
 use crate::private::PingType;
 use crate::private::{BooleanMetric, CounterMetric, EventMetric};
-use std::path::PathBuf;
 
 use super::*;
 use crate::common_test::{lock_test, new_glean, GLOBAL_APPLICATION_ID};
@@ -35,7 +34,7 @@ fn send_a_ping() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,
@@ -144,7 +143,7 @@ fn test_experiments_recording_before_glean_inits() {
     set_experiment_inactive("experiment_preinit_disabled".to_string());
 
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     test_reset_glean(
         Configuration {
@@ -205,7 +204,7 @@ fn sending_of_foreground_background_pings() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,
@@ -286,7 +285,7 @@ fn sending_of_startup_baseline_ping() {
     }
 
     // Create a custom configuration to use a fake uploader.
-    let tmpname = data_dir.path().display().to_string();
+    let tmpname = data_dir.path().to_path_buf();
 
     // Now reset Glean: it should still send a baseline ping with reason
     // dirty_startup when starting, because of the dirty bit being set.
@@ -346,7 +345,7 @@ fn no_dirty_baseline_on_clean_shutdowns() {
     }
 
     // Create a custom configuration to use a fake uploader.
-    let tmpname = data_dir.path().display().to_string();
+    let tmpname = data_dir.path().to_path_buf();
 
     // Now reset Glean: it should not send a baseline ping, because
     // we cleared the dirty bit.
@@ -376,14 +375,14 @@ fn initialize_must_not_crash_if_data_dir_is_messed_up() {
     let _lock = lock_test();
 
     let dir = tempfile::tempdir().unwrap();
-    let tmpdirname = dir.path().display().to_string();
+    let tmpdirname = dir.path();
     // Create a file in the temporary dir and use that as the
     // name of the Glean data dir.
-    let file_path = PathBuf::from(tmpdirname).join("notadir");
+    let file_path = tmpdirname.to_path_buf().join("notadir");
     std::fs::write(file_path.clone(), "test").expect("The test Glean dir file must be created");
 
     let cfg = Configuration {
-        data_path: file_path.to_string_lossy().to_string(),
+        data_path: file_path,
         application_id: GLOBAL_APPLICATION_ID.into(),
         upload_enabled: true,
         max_events: None,
@@ -442,7 +441,7 @@ fn initializing_twice_is_a_noop() {
     let _lock = lock_test();
 
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     test_reset_glean(
         Configuration {
@@ -495,7 +494,7 @@ fn the_app_channel_must_be_correctly_set_if_requested() {
     let _lock = lock_test();
 
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     // No appChannel must be set if nothing was provided through the config
     // options.
@@ -612,7 +611,7 @@ fn sending_deletion_ping_if_disabled_outside_of_run() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname.clone(),
@@ -677,7 +676,7 @@ fn no_sending_of_deletion_ping_if_unchanged_outside_of_run() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname.clone(),
@@ -760,7 +759,7 @@ fn setting_debug_view_tag_before_initialization_should_not_crash() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,
@@ -819,7 +818,7 @@ fn setting_source_tags_before_initialization_should_not_crash() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,
@@ -885,7 +884,7 @@ fn flipping_upload_enabled_respects_order_of_events() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,
@@ -955,7 +954,7 @@ fn registering_pings_before_init_must_work() {
 
     // Create a custom configuration to use a fake uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,

glean-core/rlb/tests/init_fails.rs

@@ -57,7 +57,7 @@ fn init_fails() {
 
     // Create a custom configuration to use a validating uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,

glean-core/rlb/tests/no_time_to_init.rs

@@ -55,7 +55,7 @@ fn init_fails() {
 
     // Create a custom configuration to use a validating uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,

glean-core/rlb/tests/overflowing_preinit.rs

@@ -60,7 +60,7 @@ fn overflowing_the_task_queue_records_telemetry() {
 
     // Create a custom configuration to use a validating uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,

glean-core/rlb/tests/schema.rs

@@ -22,7 +22,7 @@ const GLOBAL_APPLICATION_ID: &str = "org.mozilla.glean.test.app";
 // We need to keep the `TempDir` alive, so that it's not deleted before we stop using it.
 fn new_glean(configuration: Option<Configuration>) -> tempfile::TempDir {
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = match configuration {
         Some(c) => c,
@@ -74,7 +74,7 @@ fn validate_against_schema() {
 
     // Create a custom configuration to use a validating uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,

glean-core/rlb/tests/simple.rs

@@ -57,7 +57,7 @@ fn simple_lifecycle() {
 
     // Create a custom configuration to use a validating uploader.
     let dir = tempfile::tempdir().unwrap();
-    let tmpname = dir.path().display().to_string();
+    let tmpname = dir.path().to_path_buf();
 
     let cfg = Configuration {
         data_path: tmpname,