From 14ee567962488fe55fdc2d334348d912c9fa4205 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 25 Mar 2018 14:59:35 +0200 Subject: [PATCH] feat: Switch over to ordered maps --- Cargo.toml | 3 +- src/lib.rs | 1 + src/protocol/v7.rs | 95 +++++++++++++++++++++++---------------- tests/test_protocol_v7.rs | 71 ++++++++++++++++++++++++----- 4 files changed, 120 insertions(+), 50 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0a79b1f7..23ddd098 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,8 @@ failure_derive = "0.1.1" url = "1.7.0" serde = "1.0.34" serde_derive = "1.0.34" -serde_json = "1.0" +serde_json = { version = "1.0", features = ["preserve_order"] } url_serde = "0.2.0" chrono = { version = "0.4.0", features = ["serde"] } uuid = { version = "0.6.2", features = ["serde", "v4"] } +linked-hash-map = { version = "0.5.1", features = ["serde_impl"] } diff --git a/src/lib.rs b/src/lib.rs index 5b7eeaa9..cda956ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,7 @@ extern crate serde_json; extern crate url; extern crate url_serde; extern crate uuid; +extern crate linked_hash_map; #[macro_use] mod macros; diff --git a/src/protocol/v7.rs b/src/protocol/v7.rs index 2fe70f95..4a7e867c 100644 --- a/src/protocol/v7.rs +++ b/src/protocol/v7.rs @@ -5,7 +5,6 @@ //! a future sentry protocol will be a cleanup of the old one and is mapped //! to similar values on the rust side. use std::fmt; -use std::collections::HashMap; use std::net::IpAddr; use chrono; @@ -17,8 +16,26 @@ use serde::de::{Deserialize, Deserializer, Error as DeError}; use serde::ser::{Error as SerError, Serialize, SerializeMap, Serializer}; use serde_json::{from_value, to_value}; +/// Internals for the protocol v7's arbitrary data type. +pub mod value { + pub use serde_json::value::{Value, Index, Number, from_value, to_value}; +} + +/// Internals for the protocol v7's map type. +/// +/// It is currently backed by the `linked-hash-map` crate's hash map so that +/// insertion order is preserved. +pub mod map { + pub use linked_hash_map::{Entries, IntoIter, Iter, IterMut, Keys, + LinkedHashMap, OccupiedEntry, + VacantEntry, Values}; +} + /// An arbitrary (JSON) value. -pub use serde_json::Value; +pub use self::value::Value; + +/// The internally use arbitrary data map type. +pub use self::map::LinkedHashMap as Map; /// Represents a log entry message. /// @@ -71,8 +88,8 @@ pub struct Frame { #[serde(skip_serializing_if = "Option::is_none")] pub in_app: Option, /// Optional local variables. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub vars: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub vars: Map, /// Optional instruction information for native languages. #[serde(flatten)] pub instruction_info: InstructionInfo, @@ -285,8 +302,8 @@ pub struct Breadcrumb { #[serde(skip_serializing_if = "Option::is_none")] pub message: Option, /// Arbitrary breadcrumb data that should be send along. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub data: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub data: Map, } impl Default for Breadcrumb { @@ -297,7 +314,7 @@ impl Default for Breadcrumb { category: None, level: Level::Info, message: None, - data: HashMap::with_capacity(0), + data: Map::new(), } } } @@ -320,7 +337,7 @@ pub struct User { pub username: Option, /// Additional data that should be send along. #[serde(flatten)] - pub data: HashMap, + pub data: Map, } /// Represents http request data. @@ -344,14 +361,14 @@ pub struct Request { #[serde(skip_serializing_if = "Option::is_none")] pub cookies: Option, /// HTTP request headers. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub headers: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub headers: Map, /// Optionally a CGI/WSGI etc. environment dictionary. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub env: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub env: Map, /// Additional unhandled keys. #[serde(flatten)] - pub other: HashMap, + pub other: Map, } /// Holds information about the system SDK. @@ -379,7 +396,7 @@ pub enum DebugImage { /// A reference to a proguard debug file. Proguard(ProguardDebugImage), /// A debug image that is unknown to this protocol specification. - Unknown(HashMap), + Unknown(Map), } impl DebugImage { @@ -489,8 +506,8 @@ pub struct Event { #[serde(skip_serializing_if = "Option::is_none")] pub logger: Option, /// Optionally a name to version mapping of installed modules. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub modules: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub modules: Map, /// A platform identifier for this event. #[serde(skip_serializing_if = "is_other")] pub platform: String, @@ -506,8 +523,8 @@ pub struct Event { #[serde(skip_serializing_if = "Option::is_none")] pub release: Option, /// Repository references - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub repos: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub repos: Map, /// An optional distribution identifer. #[serde(skip_serializing_if = "Option::is_none")] pub dist: Option, @@ -521,9 +538,9 @@ pub struct Event { #[serde(skip_serializing_if = "Option::is_none")] pub request: Option, /// Optional contexts. - #[serde(skip_serializing_if = "HashMap::is_empty", serialize_with = "serialize_context", + #[serde(skip_serializing_if = "Map::is_empty", serialize_with = "serialize_context", deserialize_with = "deserialize_context")] - pub contexts: HashMap, + pub contexts: Map, /// List of breadcrumbs to send along. #[serde(skip_serializing_if = "Vec::is_empty")] pub breadcrumbs: Vec, @@ -542,11 +559,11 @@ pub struct Event { deserialize_with = "deserialize_threads")] pub threads: Vec, /// Optional tags to be attached to the event. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub tags: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub tags: Map, /// Optional extra information to be sent with the event. - #[serde(skip_serializing_if = "HashMap::is_empty")] - pub extra: HashMap, + #[serde(skip_serializing_if = "Map::is_empty")] + pub extra: Map, /// Debug meta information. #[serde(skip_serializing_if = "Option::is_none")] pub debug_meta: Option, @@ -555,7 +572,7 @@ pub struct Event { pub sdk_info: Option, /// Additional arbitrary keys for forwards compatibility. #[serde(flatten)] - pub other: HashMap, + pub other: Map, } fn is_other(value: &str) -> bool { @@ -576,27 +593,27 @@ impl Default for Event { message: None, logentry: None, logger: None, - modules: HashMap::with_capacity(0), + modules: Map::new(), platform: "other".into(), timestamp: None, server_name: None, release: None, - repos: HashMap::with_capacity(0), + repos: Map::new(), dist: None, environment: None, user: None, request: None, - contexts: HashMap::with_capacity(0), + contexts: Map::new(), breadcrumbs: Vec::new(), exceptions: Vec::new(), stacktrace: None, template_info: None, threads: Vec::new(), - tags: HashMap::with_capacity(0), - extra: HashMap::with_capacity(0), + tags: Map::new(), + extra: Map::new(), debug_meta: None, sdk_info: None, - other: HashMap::with_capacity(0), + other: Map::new(), } } } @@ -627,7 +644,7 @@ pub struct Context { /// Typed context data. pub data: ContextType, /// Additional keys sent along not known to the context type. - pub extra: HashMap, + pub extra: Map, } /// Typed contextual data @@ -705,7 +722,7 @@ impl From for Context { fn from(data: ContextType) -> Context { Context { data: data, - extra: HashMap::with_capacity(0), + extra: Map::new(), } } } @@ -739,17 +756,17 @@ where } } -fn deserialize_context<'de, D>(deserializer: D) -> Result, D::Error> +fn deserialize_context<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, { - let raw = >::deserialize(deserializer)?; - let mut rv = HashMap::new(); + let raw = >::deserialize(deserializer)?; + let mut rv = Map::new(); #[derive(Deserialize)] pub struct Helper { #[serde(flatten)] data: T, - #[serde(flatten)] extra: HashMap, + #[serde(flatten)] extra: Map, } for (key, mut raw_context) in raw { @@ -795,7 +812,7 @@ where Ok(rv) } -fn serialize_context(value: &HashMap, serializer: S) -> Result +fn serialize_context(value: &Map, serializer: S) -> Result where S: Serializer, { @@ -897,7 +914,7 @@ impl<'de> Deserialize<'de> for DebugImage { DebugImage::Proguard(img) } Some(ty) => { - let mut img: HashMap = map.into_iter().collect(); + let mut img: Map = map.into_iter().collect(); img.insert("type".into(), ty.into()); DebugImage::Unknown(img) } diff --git a/tests/test_protocol_v7.rs b/tests/test_protocol_v7.rs index bef3b97e..397e7dbd 100644 --- a/tests/test_protocol_v7.rs +++ b/tests/test_protocol_v7.rs @@ -2,8 +2,9 @@ extern crate sentry_types; extern crate serde; extern crate serde_json; extern crate uuid; +extern crate chrono; -use std::collections::HashMap; +use chrono::{TimeZone, Utc}; use sentry_types::protocol::v7; @@ -144,7 +145,7 @@ fn test_logentry_basics() { fn test_modules() { let event = v7::Event { modules: { - let mut m = HashMap::new(); + let mut m = v7::Map::new(); m.insert("System".into(), "1.0.0".into()); m }, @@ -160,7 +161,7 @@ fn test_modules() { fn test_repos() { let event = v7::Event { repos: { - let mut m = HashMap::new(); + let mut m = v7::Map::new(); m.insert("/raven".into(), v7::RepoReference { name: "github/raven".into(), prefix: Some("/".into()), @@ -178,7 +179,7 @@ fn test_repos() { let event = v7::Event { repos: { - let mut m = HashMap::new(); + let mut m = v7::Map::new(); m.insert("/raven".into(), v7::RepoReference { name: "github/raven".into(), prefix: None, @@ -195,6 +196,20 @@ fn test_repos() { ); } +#[test] +fn test_platform_and_timestamp() { + let event = v7::Event { + platform: "python".into(), + timestamp: Some(Utc.ymd(2017, 12, 24).and_hms(8, 12, 0)), + ..Default::default() + }; + + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"platform\":\"python\",\"timestamp\":\"2017-12-24T08:12:00Z\"}" + ); +} + #[test] fn test_user() { let event = v7::Event { @@ -204,7 +219,7 @@ fn test_user() { ip_address: Some("127.0.0.1".parse().unwrap()), username: Some("john-doe".into()), data: { - let mut hm = HashMap::new(); + let mut hm = v7::Map::new(); hm.insert("foo".into(), "bar".into()); hm } @@ -236,6 +251,42 @@ fn test_user() { ); } +#[test] +fn test_breadcrumbs() { + let event = v7::Event { + breadcrumbs: vec![ + v7::Breadcrumb { + timestamp: Utc.ymd(2017, 12, 24).and_hms_milli(8, 12, 0, 713), + category: Some("ui.click".into()), + message: Some("span.platform-card > li.platform-tile".into()), + ..Default::default() + }, + v7::Breadcrumb { + timestamp: Utc.ymd(2017, 12, 24).and_hms_milli(8, 12, 0, 913), + ty: "http".into(), + category: Some("xhr".into()), + data: { + let mut m = v7::Map::new(); + m.insert("url".into(), "/api/0/organizations/foo".into()); + m.insert("status_code".into(), 200.into()); + m.insert("method".into(), "GET".into()); + m + }, + ..Default::default() + }, + ], + ..Default::default() + }; + + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"breadcrumbs\":[{\"timestamp\":1514103120,\"type\":\"default\",\ + \"category\":\"ui.click\",\"message\":\"span.platform-card > li.platform-tile\"}\ + ,{\"timestamp\":1514103120,\"type\":\"http\",\"category\":\"xhr\",\"data\":\ + {\"url\":\"/api/0/organizations/foo\",\"status_code\":200,\"method\":\"GET\"}}]}" + ); +} + #[test] fn test_request() { let event = v7::Event { @@ -246,12 +297,12 @@ fn test_request() { query_string: Some("foo=bar&blub=blah".into()), cookies: Some("dummy=42".into()), headers: { - let mut hm = HashMap::new(); + let mut hm = v7::Map::new(); hm.insert("Content-Type".into(), "text/plain".into()); hm }, env: { - let mut env = HashMap::new(); + let mut env = v7::Map::new(); env.insert("PATH_INFO".into(), "/bar".into()); env }, @@ -277,7 +328,7 @@ fn test_request() { query_string: Some("foo=bar&blub=blah".into()), cookies: Some("dummy=42".into()), other: { - let mut m = HashMap::new(); + let mut m = v7::Map::new(); m.insert("other_key".into(), "other_value".into()); m }, @@ -404,7 +455,7 @@ fn test_slightly_larger_exception_stacktrace() { }, in_app: Some(true), vars: { - let mut m = HashMap::new(); + let mut m = v7::Map::new(); m.insert("var".into(), "value".into()); m }, @@ -453,7 +504,7 @@ fn test_full_exception_stacktrace() { }, in_app: Some(true), vars: { - let mut m = HashMap::new(); + let mut m = v7::Map::new(); m.insert("var".into(), "value".into()); m },