From 29fb733b2f57e55e13d4d1c83962c16577816217 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Mon, 10 May 2021 15:01:37 -0700 Subject: [PATCH] Remove anymap `anymap` has some undefined behavior, and appears to no longer be maintained. This patch replaces anymap with an opaque `EventAttributes` type, which contains a number of accessor methods to get and set values. Under the covers, this type is simply a box of a struct of option types. However, since it's opaque the underlying type could be swapped out with an alternative type that can be extended to support more attributes without breaking users. Closes #306 --- Cargo.toml | 1 - src/event.rs | 300 +++++++++++++++++++------------------- src/fsevent.rs | 2 +- src/lib.rs | 2 - tests/serialise-events.rs | 36 +++-- 5 files changed, 173 insertions(+), 168 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6a7a8e57..191b2556 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,6 @@ exclude = [ ] [dependencies] -anymap = "0.12.1" bitflags = "1.0.4" crossbeam-channel = "0.5.0" filetime = "0.2.6" diff --git a/src/event.rs b/src/event.rs index ca3a40f7..3028e218 100644 --- a/src/event.rs +++ b/src/event.rs @@ -2,7 +2,6 @@ // LICENSE.ARTISTIC file, and the Creative Commons Zero 1.0 license. //! The `Event` type and the hierarchical `EventKind` descriptor. -use anymap::{any::CloneAny, Map}; use std::{ fmt, hash::{Hash, Hasher}, @@ -12,9 +11,6 @@ use std::{ #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -/// An `AnyMap` convenience type with the needed bounds for events. -pub type AnyMap = Map; - /// An event describing open or close operations on files. #[derive(Clone, Debug, Eq, Hash, PartialEq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -360,28 +356,143 @@ pub struct Event { /// Arbitrary data may be added to this field, without restriction beyond the `Sync` and /// `Clone` properties. Some data added here is considered for comparing and hashing, but not /// all: at this writing this is `Tracker`, `Flag`, `Info`, and `Source`. + #[cfg_attr(feature = "serde", serde(default))] + pub attrs: EventAttributes, +} + +/// Additional attributes of the event. +#[derive(Clone, Default)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct EventAttributes { + #[cfg_attr(feature = "serde", serde(flatten))] + inner: Option>, +} + +#[derive(Clone, Default)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +struct EventAttributesInner { + /// Tracking ID for events that are related. + /// + /// For events generated by backends with the `TrackRelated` capability. Those backends _may_ + /// emit events that are related to each other, and tag those with an identical "tracking id" + /// or "cookie". The value is normalised to `usize`. + #[cfg_attr( + feature = "serde", + serde(default, skip_serializing_if = "Option::is_none") + )] + tracker: Option, + + /// Special Notify flag on the event. + #[cfg_attr( + feature = "serde", + serde(default, skip_serializing_if = "Option::is_none") + )] + flag: Option, + + /// Additional information on the event. + /// + /// This is to be used for all `Other` variants of the event kind hierarchy. The variant + /// indicates that a consumer should look into the `attrs` for an `Info` value; if that value + /// is missing it should be considered a backend bug. + /// + /// This attribute may also be present for non-`Other` variants of the event kind, if doing so + /// provides useful precision. For example, the `Modify(Metadata(Extended))` kind suggests + /// using this attribute when information about _what_ extended metadata changed is available. /// - /// For vendor or custom information, it is recommended to use type wrappers to differentiate - /// entries within the `AnyMap` container and avoid conflicts. For interoperability, one of the - /// “well-known” types (or propose a new one) should be used instead. See the list on the wiki: - /// https://github.com/notify-rs/notify/wiki/Well-Known-Event-Attrs + /// This should be a short string, and changes may be considered breaking. + #[cfg_attr( + feature = "serde", + serde(default, skip_serializing_if = "Option::is_none") + )] + info: Option, + + /// The source of the event. /// - /// There is currently no way to serialise or deserialise arbitrary attributes, only well-known - /// ones handled explicitly here are supported. This might change in the future. - #[cfg_attr(feature = "serde", serde(default = "AnyMap::new"))] - #[cfg_attr(feature = "serde", serde(deserialize_with = "attr_serde::deserialize"))] - #[cfg_attr(feature = "serde", serde(serialize_with = "attr_serde::serialize"))] - pub attrs: AnyMap, + /// In most cases this should be a short string, identifying the backend unambiguously. In some + /// cases this may be dynamically generated, but should contain a prefix to make it unambiguous + /// between backends. + #[cfg_attr( + feature = "serde", + serde(default, skip_serializing_if = "Option::is_none") + )] + source: Option, + + /// The process ID of the originator of the event. + /// + /// This attribute is experimental and, while included in Notify itself, is not considered + /// stable or standard enough to be part of the serde, eq, hash, and debug representations. + #[cfg_attr( + feature = "serde", + serde(default, skip_serializing, skip_deserializing) + )] + process_id: Option, } -/// Tracking ID for events that are related. -/// -/// For events generated by backends with the `TrackRelated` capability. Those backends _may_ emit -/// events that are related to each other, and tag those with an identical "tracking id" or -/// "cookie". The value is normalised to `usize`. -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] -#[cfg_attr(feature = "serde", derive(Deserialize))] -pub struct Tracker(pub usize); +impl EventAttributes { + /// Creates a new `EventAttributes`. + pub fn new() -> Self { + Self { inner: None } + } + + /// Retrieves the tracker ID for an event directly, if present. + pub fn tracker(&self) -> Option { + self.inner.as_ref().and_then(|inner| inner.tracker.clone()) + } + + /// Retrieves the Notify flag for an event directly, if present. + pub fn flag(&self) -> Option { + self.inner.as_ref().and_then(|inner| inner.flag.clone()) + } + + /// Retrieves the additional info for an event directly, if present. + pub fn info(&self) -> Option<&str> { + self.inner + .as_ref() + .and_then(|inner| inner.info.as_ref().map(|info| &**info)) + } + + /// Retrieves the source for an event directly, if present. + pub fn source(&self) -> Option<&str> { + self.inner + .as_ref() + .and_then(|inner| inner.source.as_ref().map(|source| &**source)) + } + + /// The process ID of the originator of the event. + /// + /// This attribute is experimental and, while included in Notify itself, is not considered + /// stable or standard enough to be part of the serde, eq, hash, and debug representations. + pub fn process_id(&self) -> Option { + self.inner + .as_ref() + .and_then(|inner| inner.process_id.clone()) + } + + /// Sets the tracker. + pub fn set_tracker(&mut self, tracker: usize) { + self.inner_mut().tracker = Some(tracker); + } + + /// Sets the Notify flag onto the event. + pub fn set_flag(&mut self, flag: Flag) { + self.inner_mut().flag = Some(flag); + } + + /// Sets additional info onto the event. + pub fn set_info(&mut self, info: &str) { + self.inner_mut().info = Some(info.to_string()); + } + + /// Sets the process id onto the event. + pub fn set_process_id(&mut self, process_id: u32) { + self.inner_mut().process_id = Some(process_id) + } + + fn inner_mut(&mut self) -> &mut EventAttributesInner { + self.inner + .get_or_insert_with(|| Box::new(Default::default())) + } +} /// Special Notify flag on the event. /// @@ -413,59 +524,25 @@ pub enum Flag { Rescan, } -/// Additional information on the event. -/// -/// This is to be used for all `Other` variants of the event kind hierarchy. The variant indicates -/// that a consumer should look into the `attrs` for an `Info` value; if that value is missing it -/// should be considered a backend bug. -/// -/// This attribute may also be present for non-`Other` variants of the event kind, if doing so -/// provides useful precision. For example, the `Modify(Metadata(Extended))` kind suggests using -/// this attribute when information about _what_ extended metadata changed is available. -/// -/// This should be a short string, and changes may be considered breaking. -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] -#[cfg_attr(feature = "serde", derive(Deserialize))] -pub struct Info(pub String); - -/// The source of the event. -/// -/// In most cases this should be a short string, identifying the backend unambiguously. In some -/// cases this may be dynamically generated, but should contain a prefix to make it unambiguous -/// between backends. -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] -#[cfg_attr(feature = "serde", derive(Deserialize))] -pub struct Source(pub String); - -/// The process ID of the originator of the event. -/// -/// This attribute is experimental and, while included in Notify itself, is not considered stable -/// or standard enough to be part of the serde, eq, hash, and debug representations. -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] -#[cfg_attr(feature = "serde", derive(Deserialize))] -pub struct ProcessID(pub u32); - -// + typeid attr? - impl Event { /// Retrieves the tracker ID for an event directly, if present. pub fn tracker(&self) -> Option { - self.attrs.get::().map(|v| v.0) + self.attrs.tracker() } /// Retrieves the Notify flag for an event directly, if present. pub fn flag(&self) -> Option { - self.attrs.get::().cloned() + self.attrs.flag() } /// Retrieves the additional info for an event directly, if present. - pub fn info(&self) -> Option<&String> { - self.attrs.get::().map(|v| &v.0) + pub fn info(&self) -> Option<&str> { + self.attrs.info() } /// Retrieves the source for an event directly, if present. - pub fn source(&self) -> Option<&String> { - self.attrs.get::().map(|v| &v.0) + pub fn source(&self) -> Option<&str> { + self.attrs.source() } /// Creates a new `Event` given a kind. @@ -473,7 +550,7 @@ impl Event { Self { kind, paths: Vec::new(), - attrs: AnyMap::new(), + attrs: EventAttributes::new(), } } @@ -500,19 +577,25 @@ impl Event { /// Sets the tracker. pub fn set_tracker(mut self, tracker: usize) -> Self { - self.attrs.insert(Tracker(tracker)); + self.attrs.set_tracker(tracker); self } /// Sets additional info onto the event. pub fn set_info(mut self, info: &str) -> Self { - self.attrs.insert(Info(info.into())); + self.attrs.set_info(info); self } /// Sets the Notify flag onto the event. pub fn set_flag(mut self, flag: Flag) -> Self { - self.attrs.insert(flag); + self.attrs.set_flag(flag); + self + } + + /// Sets the process id onto the event. + pub fn set_process_id(mut self, process_id: u32) -> Self { + self.attrs.set_process_id(process_id); self } } @@ -534,7 +617,7 @@ impl Default for Event { Self { kind: EventKind::default(), paths: Vec::new(), - attrs: AnyMap::new(), + attrs: EventAttributes::new(), } } } @@ -561,86 +644,3 @@ impl Hash for Event { self.source().hash(state); } } - -#[cfg(feature = "serde")] -mod attr_serde { - use super::*; - use serde::{ - de::Deserializer, - ser::{SerializeMap, Serializer}, - }; - use std::collections::HashMap; - - pub(crate) fn serialize(attrs: &AnyMap, s: S) -> Result - where - S: Serializer, - { - let tracker = attrs.get::().map(|v| v.0.clone()); - let flag = attrs.get::().map(|v| v.clone()); - let info = attrs.get::().map(|v| v.0.clone()); - let source = attrs.get::().map(|v| v.0.clone()); - - let mut length = 0; - if tracker.is_some() { - length += 1; - } - if flag.is_some() { - length += 1; - } - if info.is_some() { - length += 1; - } - if source.is_some() { - length += 1; - } - - let mut map = s.serialize_map(Some(length))?; - if let Some(val) = tracker { - map.serialize_entry("tracker", &val)?; - } - if let Some(val) = flag { - map.serialize_entry("flag", &val)?; - } - if let Some(val) = info { - map.serialize_entry("info", &val)?; - } - if let Some(val) = source { - map.serialize_entry("source", &val)?; - } - map.end() - } - - pub(crate) fn deserialize<'de, D>(d: D) -> Result - where - D: Deserializer<'de>, - { - #[derive(Clone, Deserialize)] - #[serde(rename_all = "kebab-case")] - #[serde(untagged)] - enum Attr { - Tracker(Option), - Flag(Option), - Info(Option), - Source(Option), - } - - #[derive(Deserialize)] - struct Attrs(pub HashMap); - - let attrs = Attrs::deserialize(d)?; - let mut map = AnyMap::with_capacity(attrs.0.len()); - if let Some(Attr::Tracker(Some(val))) = attrs.0.get("tracker").cloned() { - map.insert(val); - } - if let Some(Attr::Flag(Some(val))) = attrs.0.get("flag").cloned() { - map.insert(val); - } - if let Some(Attr::Info(Some(val))) = attrs.0.get("info").cloned() { - map.insert(val); - } - if let Some(Attr::Source(Some(val))) = attrs.0.get("source").cloned() { - map.insert(val); - } - Ok(map) - } -} diff --git a/src/fsevent.rs b/src/fsevent.rs index bb4ac867..87f23bc8 100644 --- a/src/fsevent.rs +++ b/src/fsevent.rs @@ -192,7 +192,7 @@ fn translate_flags(flags: fse::StreamFlags, precise: bool) -> Vec { if flags.contains(fse::StreamFlags::OWN_EVENT) { for ev in &mut evs { - ev.attrs.insert(ProcessID(std::process::id())); + *ev = std::mem::replace(ev, Event::default()).set_process_id(std::process::id()); } } diff --git a/src/lib.rs b/src/lib.rs index 3747f716..872c59f8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,8 +95,6 @@ //! # } //! ``` -// FIXME: `anymap` crate triggers this lint and we cannot do anything here. -#![allow(where_clauses_object_safety)] #![deny(missing_docs)] pub use config::{Config, RecursiveMode}; diff --git a/tests/serialise-events.rs b/tests/serialise-events.rs index daccf594..bc2c986e 100644 --- a/tests/serialise-events.rs +++ b/tests/serialise-events.rs @@ -1,9 +1,6 @@ // This file is dual-licensed under the Artistic License 2.0 as per the // LICENSE.ARTISTIC file, and the Creative Commons Zero 1.0 license. -// FIXME: `anymap` crate triggers this lint and we cannot do anything here. -#![allow(where_clauses_object_safety)] - use notify::event::*; #[cfg(feature = "serde")] use serde_json::json; @@ -20,9 +17,9 @@ fn events_are_debuggable() { String::from("Access(Open(Execute))") ); - let mut attrs = AnyMap::new(); - attrs.insert(Info("unmount".into())); - attrs.insert(Flag::Rescan); + let mut attrs = EventAttributes::new(); + attrs.set_info("unmount".into()); + attrs.set_flag(Flag::Rescan); assert_eq!( format!( @@ -50,7 +47,7 @@ fn events_are_serializable() { json!(Event { kind: EventKind::Access(AccessKind::Open(AccessMode::Execute)), paths: Vec::new(), - attrs: AnyMap::new(), + attrs: EventAttributes::new(), }), json!({ "type": { "access": { "kind": "open", "mode": "execute" } }, @@ -59,20 +56,31 @@ fn events_are_serializable() { }) ); - let mut attrs = AnyMap::new(); - attrs.insert(Info("unmount".into())); + let mut attrs = EventAttributes::new(); + attrs.set_info("unmount".into()); assert_eq!( json!(Event { kind: EventKind::Remove(RemoveKind::Other), paths: vec!["/example".into()], - attrs + attrs: attrs.clone(), }), json!({ "type": { "remove": { "kind": "other" } }, "paths": ["/example"], "attrs": { "info": "unmount" } - }) + }), + "{:#?} != {:#?}", + json!(Event { + kind: EventKind::Remove(RemoveKind::Other), + paths: vec!["/example".into()], + attrs: attrs.clone(), + }), + json!({ + "type": { "remove": { "kind": "other" } }, + "paths": ["/example"], + "attrs": { "info": "unmount" } + }), ); } @@ -101,12 +109,12 @@ fn events_are_deserializable() { Event { kind: EventKind::Access(AccessKind::Open(AccessMode::Execute)), paths: Vec::new(), - attrs: AnyMap::new(), + attrs: EventAttributes::new(), } ); - let mut attrs = AnyMap::new(); - attrs.insert(Info("unmount".into())); + let mut attrs = EventAttributes::new(); + attrs.set_info("unmount".into()); assert_eq!( serde_json::from_str::(