Change the resource ids to be a Set rather than a Vec (#277)

Author: gregtatum

fluent-fallback/Cargo.toml

@@ -24,6 +24,7 @@ futures = "0.3"
 async-trait = "0.1"
 unic-langid = { version = "0.9" }
 once_cell = "1.9"
+rustc-hash = "1"
 
 [dev-dependencies]
 fluent-langneg = "0.13"

fluent-fallback/examples/simple-fallback.rs

@@ -28,6 +28,7 @@ use fluent_fallback::{
 };
 use fluent_langneg::{negotiate_languages, NegotiationStrategy};
 
+use rustc_hash::FxHashSet;
 use unic_langid::{langid, LanguageIdentifier};
 
 /// This helper struct holds the scheme for converting
@@ -111,7 +112,7 @@ fn main() {
     let bundles = get_resource_manager();
 
     let loc = Localization::with_env(
-        L10N_RESOURCES.iter().map(|&res| res.into()).collect(),
+        L10N_RESOURCES.iter().map(|&res| res.into()),
         true,
         app_locales,
         bundles,
@@ -168,7 +169,7 @@ fn collatz(n: isize) -> isize {
 struct BundleIter {
     res_path_scheme: String,
     locales: <Vec<LanguageIdentifier> as IntoIterator>::IntoIter,
-    res_ids: Vec<ResourceId>,
+    res_ids: FxHashSet<ResourceId>,
 }
 
 impl Iterator for BundleIter {
@@ -225,7 +226,7 @@ impl BundleGenerator for Bundles {
     fn bundles_iter(
         &self,
         locales: std::vec::IntoIter<LanguageIdentifier>,
-        res_ids: Vec<ResourceId>,
+        res_ids: FxHashSet<ResourceId>,
     ) -> Self::Iter {
         BundleIter {
             res_path_scheme: self.res_path_scheme.to_string_lossy().to_string(),

fluent-fallback/src/bundles.rs

@@ -6,6 +6,7 @@ use crate::{
     types::{L10nAttribute, L10nKey, L10nMessage, ResourceId},
 };
 use fluent_bundle::{FluentArgs, FluentBundle, FluentError};
+use rustc_hash::FxHashSet;
 use std::borrow::Cow;
 
 pub enum BundlesInner<G>
@@ -50,7 +51,7 @@ impl<G> Bundles<G>
 where
     G: BundleGenerator,
 {
-    pub fn new<P>(sync: bool, res_ids: Vec<ResourceId>, generator: &G, provider: &P) -> Self
+    pub fn new<P>(sync: bool, res_ids: FxHashSet<ResourceId>, generator: &G, provider: &P) -> Self
     where
         G: BundleGenerator<LocalesIter = P::Iter>,
         P: LocalesProvider,

fluent-fallback/src/generator.rs

@@ -1,5 +1,6 @@
 use fluent_bundle::{FluentBundle, FluentError, FluentResource};
 use futures::Stream;
+use rustc_hash::FxHashSet;
 use std::borrow::Borrow;
 use unic_langid::LanguageIdentifier;
 
@@ -22,14 +23,18 @@ pub trait BundleGenerator {
     type Iter: Iterator<Item = FluentBundleResult<Self::Resource>>;
     type Stream: Stream<Item = FluentBundleResult<Self::Resource>>;
 
-    fn bundles_iter(&self, _locales: Self::LocalesIter, _res_ids: Vec<ResourceId>) -> Self::Iter {
+    fn bundles_iter(
+        &self,
+        _locales: Self::LocalesIter,
+        _res_ids: FxHashSet<ResourceId>,
+    ) -> Self::Iter {
         unimplemented!();
     }
 
     fn bundles_stream(
         &self,
         _locales: Self::LocalesIter,
-        _res_ids: Vec<ResourceId>,
+        _res_ids: FxHashSet<ResourceId>,
     ) -> Self::Stream {
         unimplemented!();
     }

fluent-fallback/src/localization.rs

@@ -5,6 +5,7 @@ use crate::{
     types::ResourceId,
 };
 use once_cell::sync::OnceCell;
+use rustc_hash::FxHashSet;
 use std::rc::Rc;
 
 pub struct Localization<G, P>
@@ -16,21 +17,24 @@ where
     generator: G,
     provider: P,
     sync: bool,
-    res_ids: Vec<ResourceId>,
+    res_ids: FxHashSet<ResourceId>,
 }
 
 impl<G, P> Localization<G, P>
 where
     G: BundleGenerator<LocalesIter = P::Iter> + Default,
     P: LocalesProvider + Default,
 {
-    pub fn new(res_ids: Vec<ResourceId>, sync: bool) -> Self {
+    pub fn new<I>(res_ids: I, sync: bool) -> Self
+    where
+        I: IntoIterator<Item = ResourceId>,
+    {
         Self {
             bundles: OnceCell::new(),
             generator: G::default(),
             provider: P::default(),
             sync,
-            res_ids,
+            res_ids: FxHashSet::from_iter(res_ids.into_iter()),
         }
     }
 }
@@ -40,13 +44,16 @@ where
     G: BundleGenerator<LocalesIter = P::Iter>,
     P: LocalesProvider,
 {
-    pub fn with_env(res_ids: Vec<ResourceId>, sync: bool, provider: P, generator: G) -> Self {
+    pub fn with_env<I>(res_ids: I, sync: bool, provider: P, generator: G) -> Self
+    where
+        I: IntoIterator<Item = ResourceId>,
+    {
         Self {
             bundles: OnceCell::new(),
             generator,
             provider,
             sync,
-            res_ids,
+            res_ids: FxHashSet::from_iter(res_ids.into_iter()),
         }
     }
 
@@ -55,7 +62,7 @@ where
     }
 
     pub fn add_resource_id<T: Into<ResourceId>>(&mut self, res_id: T) {
-        self.res_ids.push(res_id.into());
+        self.res_ids.insert(res_id.into());
         self.on_change();
     }
 

fluent-fallback/src/types.rs

@@ -28,7 +28,7 @@ pub struct L10nMessage<'l> {
     pub attributes: Vec<L10nAttribute<'l>>,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum ResourceType {
     /// This is a required resource.
     ///
@@ -53,7 +53,7 @@ pub enum ResourceType {
 }
 
 /// A resource identifier for a localization resource.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Hash)]
 pub struct ResourceId {
     /// The resource identifier.
     pub value: String,

fluent-fallback/tests/localization_test.rs

@@ -11,6 +11,7 @@ use fluent_fallback::{
     types::{L10nKey, ResourceId},
     Localization, LocalizationError,
 };
+use rustc_hash::FxHashSet;
 use std::cell::RefCell;
 use std::rc::Rc;
 use unic_langid::{langid, LanguageIdentifier};
@@ -55,7 +56,7 @@ impl LocalesProvider for Locales {
 // lack of GATs, these have to own members instead of taking slices.
 struct BundleIter {
     locales: <Vec<LanguageIdentifier> as IntoIterator>::IntoIter,
-    res_ids: Vec<ResourceId>,
+    res_ids: FxHashSet<ResourceId>,
 }
 
 impl Iterator for BundleIter {
@@ -132,11 +133,19 @@ impl BundleGenerator for ResourceManager {
     type Iter = BundleIter;
     type Stream = BundleIter;
 
-    fn bundles_iter(&self, locales: Self::LocalesIter, res_ids: Vec<ResourceId>) -> Self::Iter {
+    fn bundles_iter(
+        &self,
+        locales: Self::LocalesIter,
+        res_ids: FxHashSet<ResourceId>,
+    ) -> Self::Iter {
         BundleIter { locales, res_ids }
     }
 
-    fn bundles_stream(&self, locales: Self::LocalesIter, res_ids: Vec<ResourceId>) -> Self::Stream {
+    fn bundles_stream(
+        &self,
+        locales: Self::LocalesIter,
+        res_ids: FxHashSet<ResourceId>,
+    ) -> Self::Stream {
         BundleIter { locales, res_ids }
     }
 }
@@ -488,3 +497,22 @@ async fn localization_handle_state_changes_mid_async() {
 
     bundles.format_value("key", None, &mut errors).await;
 }
+
+#[test]
+fn localization_duplicate_resources() {
+    let resource_ids: Vec<ResourceId> =
+        vec!["test.ftl".into(), "test2.ftl".into(), "test2.ftl".into()];
+    let locales = Locales::new(vec![langid!("pl"), langid!("en-US")]);
+    let res_mgr = ResourceManager;
+    let mut errors = vec![];
+
+    let loc = Localization::with_env(resource_ids, true, locales, res_mgr);
+    let bundles = loc.bundles();
+
+    let value = bundles
+        .format_value_sync("hello-world", None, &mut errors)
+        .unwrap();
+    assert_eq!(value, Some(Cow::Borrowed("Hello World [pl]")));
+
+    assert_eq!(errors.len(), 0, "There were no errors");
+}

fluent-resmgr/Cargo.toml

@@ -22,6 +22,7 @@ fluent-fallback = { version = "0.6.0", path = "../fluent-fallback" }
 unic-langid = "0.9"
 elsa = "1.5"
 futures = "0.3"
+rustc-hash = "1"
 
 [dev-dependencies]
 unic-langid = { version = "0.9", features = ["macros"]}

fluent-resmgr/src/resource_manager.rs

@@ -5,6 +5,7 @@ use fluent_fallback::{
     types::ResourceId,
 };
 use futures::stream::Stream;
+use rustc_hash::FxHashSet;
 use std::fs;
 use std::io;
 use std::iter;
@@ -83,7 +84,7 @@ impl ResourceManager {
 // lack of GATs, these have to own members instead of taking slices.
 pub struct BundleIter {
     locales: <Vec<LanguageIdentifier> as IntoIterator>::IntoIter,
-    res_ids: Vec<ResourceId>,
+    res_ids: FxHashSet<ResourceId>,
 }
 
 impl Iterator for BundleIter {
@@ -94,7 +95,7 @@ impl Iterator for BundleIter {
 
         let mut bundle = FluentBundle::new(vec![locale.clone()]);
 
-        for res_id in &self.res_ids {
+        for res_id in self.res_ids.iter() {
             let full_path = format!("./tests/resources/{}/{}", locale, res_id);
             let source = fs::read_to_string(full_path).unwrap();
             let res = FluentResource::try_new(source).unwrap();
@@ -121,14 +122,18 @@ impl BundleGenerator for ResourceManager {
     type Iter = BundleIter;
     type Stream = BundleIter;
 
-    fn bundles_iter(&self, locales: Self::LocalesIter, res_ids: Vec<ResourceId>) -> Self::Iter {
+    fn bundles_iter(
+        &self,
+        locales: Self::LocalesIter,
+        res_ids: FxHashSet<ResourceId>,
+    ) -> Self::Iter {
         BundleIter { locales, res_ids }
     }
 
     fn bundles_stream(
         &self,
         _locales: Self::LocalesIter,
-        _res_ids: Vec<ResourceId>,
+        _res_ids: FxHashSet<ResourceId>,
     ) -> Self::Stream {
         todo!()
     }