use a PrimaryMap to avoid having to manually track the IDs for dictio…

…nary-handles

cli/src/main.rs

@@ -49,7 +49,6 @@ pub async fn serve(opts: Opts) -> Result<(), Error> {
         ctx = ctx
             .with_backends(backends.clone())
             .with_dictionaries(dictionaries.clone())
-            .with_dictionaries_by_id(dictionaries.clone())
             .with_config_path(config_path.into());
 
         if backend_names.is_empty() {

lib/src/config.rs

@@ -3,7 +3,6 @@
 use {
     self::backends::BackendsConfig,
     crate::error::FastlyConfigError,
-    crate::wiggle_abi::types::DictionaryHandle,
     serde_derive::Deserialize,
     std::{collections::HashMap, convert::TryInto, fs, path::Path, str::FromStr, sync::Arc},
     toml::value::Table,
@@ -17,8 +16,8 @@ mod unit_tests;
 mod dictionaries;
 use self::dictionaries::DictionariesConfig;
 pub use self::dictionaries::Dictionary;
-pub type Dictionaries = HashMap<String, Arc<Dictionary>>;
-pub type DictionariesById = HashMap<DictionaryHandle, Arc<Dictionary>>;
+pub use self::dictionaries::DictionaryName;
+pub type Dictionaries = HashMap<DictionaryName, Dictionary>;
 
 /// Types and deserializers for backend configuration settings.
 mod backends;
@@ -68,13 +67,6 @@ impl FastlyConfig {
         &self.local_server.dictionaries.0
     }
 
-    /// Get the backend configuration.
-    pub fn dictionaries_by_id(&self) -> DictionariesById {
-        let dicts = self.local_server.dictionaries.0.clone();
-
-        dicts.into_iter().map(|a| (a.1.id, a.1.clone())).collect()
-    }
-
     /// Parse a `fastly.toml` file into a `FastlyConfig`.
     pub fn from_file(path: impl AsRef<Path>) -> Result<Self, FastlyConfigError> {
         fs::read_to_string(path.as_ref())

lib/src/config/dictionaries.rs

@@ -1,15 +1,9 @@
 use {
-    crate::wiggle_abi::types::DictionaryHandle,
     core::fmt,
-    std::{
-        collections::HashMap,
-        sync::{atomic::AtomicU32, Arc},
-    },
+    std::collections::HashMap,
 };
 
-static COUNTER: AtomicU32 = AtomicU32::new(1);
-
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct DictionaryName(String);
 
 impl fmt::Display for DictionaryName {
@@ -18,20 +12,25 @@ impl fmt::Display for DictionaryName {
     }
 }
 
+impl DictionaryName {
+    pub fn new(name: String) -> Self {
+        Self(name)
+    }
+}
+
 /// A single Dictionary definition.
 ///
 /// A Dictionary consists of a name and an id, but more fields may be added in the future.
 #[derive(Clone, Debug)]
 pub struct Dictionary {
     // Name must start with alphabetical and contain only alphanumeric, underscore, and whitespace
     pub name: DictionaryName,
-    pub id: DictionaryHandle,
     pub file: String,
 }
 
 /// A map of [`Backend`] definitions, keyed by their name.
 #[derive(Clone, Debug, Default)]
-pub struct DictionariesConfig(pub HashMap<String, Arc<Dictionary>>);
+pub struct DictionariesConfig(pub HashMap<DictionaryName, Dictionary>);
 
 /// This module contains [`TryFrom`] implementations used when deserializing a `fastly.toml`.
 ///
@@ -41,14 +40,12 @@ pub struct DictionariesConfig(pub HashMap<String, Arc<Dictionary>>);
 mod deserialization {
 
     use {
-        super::{DictionariesConfig, Dictionary, DictionaryName, COUNTER},
+        super::{DictionariesConfig, Dictionary, DictionaryName},
         crate::error::{DictionaryConfigError, FastlyConfigError},
-        crate::wiggle_abi::types::DictionaryHandle,
         std::{
             convert::TryFrom,
             convert::TryInto,
             fs,
-            sync::{atomic::Ordering, Arc},
         },
         toml::value::{Table, Value},
         tracing::{event, Level},
@@ -85,14 +82,14 @@ mod deserialization {
             /// Process a backend's definitions, or return a [`FastlyConfigError`].
             fn process_entry(
                 (name, defs): (String, Value),
-            ) -> Result<(String, Arc<Dictionary>), FastlyConfigError> {
+            ) -> Result<(DictionaryName, Dictionary), FastlyConfigError> {
                 into_table(defs)
                     .and_then(Dictionary::try_from)
                     .map_err(|err| FastlyConfigError::InvalidDictionaryDefinition {
                         name: name.clone(),
                         err,
                     })
-                    .map(|def| (name, Arc::new(def)))
+                    .map(|def| (def.name.clone(), def))
             }
 
             toml.into_iter()
@@ -170,10 +167,8 @@ mod deserialization {
                     });
                 }
             }
-            let count = COUNTER.fetch_add(1, Ordering::SeqCst);
-            let id = DictionaryHandle::from(count);
 
-            Ok(Self { name, id, file })
+            Ok(Self { name, file })
         }
     }
 

lib/src/config/unit_tests.rs

@@ -1,8 +1,13 @@
-use std::fs::File;
-use std::io::Write;
+use {
+    crate::config::DictionaryName,
+    std::{
+        fs::File,
+        io::Write,
+    },
+    super::{FastlyConfig, LocalServerConfig, RawLocalServerConfig},
+    tempfile::tempdir,
+};
 
-use super::{FastlyConfig, LocalServerConfig, RawLocalServerConfig};
-use tempfile::tempdir;
 
 #[test]
 fn fastly_toml_files_can_be_read() {
@@ -113,7 +118,7 @@ fn fastly_toml_files_with_simple_dictionary_configurations_can_be_read() {
 
     let dictionary = config
         .dictionaries()
-        .get("a")
+        .get(&DictionaryName::new("a".to_string()))
         .expect("dictionary configurations can be accessed");
     assert_eq!(dictionary.name.to_string(), "a".to_string());
     assert_eq!(dictionary.file, file_path.to_str().unwrap());

lib/src/error.rs

@@ -1,7 +1,10 @@
 //! Error types.
 
 use {
-    crate::wiggle_abi::types::{DictionaryHandle, FastlyStatus},
+    crate::{
+        config::DictionaryName,
+        wiggle_abi::types::{DictionaryHandle, FastlyStatus}
+    },
     url::Url,
     wiggle::GuestError,
 };
@@ -85,7 +88,7 @@ pub enum Error {
     UnknownBackend(String),
 
     #[error("Unknown dictionary: {0}")]
-    UnknownDictionary(String),
+    UnknownDictionary(DictionaryName),
 
     #[error("Unknown dictionary item: {0}")]
     UnknownDictionaryItem(String),

lib/src/execute.rs

@@ -1,6 +1,6 @@
 //! Guest code execution.
 
-use crate::config::{Dictionaries, DictionariesById};
+use crate::config::Dictionaries;
 
 use {
     crate::{
@@ -40,8 +40,6 @@ pub struct ExecuteCtx {
     backends: Arc<Backends>,
     /// The dictionaries for this execution.
     dictionaries: Arc<Dictionaries>,
-    /// The dictionaries for this execution.
-    dictionaries_by_id: Arc<DictionariesById>,
     /// Path to the config, defaults to None
     config_path: Arc<Option<PathBuf>>,
     /// Whether to treat stdout as a logging endpoint
@@ -98,7 +96,6 @@ impl ExecuteCtx {
             instance_pre: Arc::new(instance_pre),
             backends: Arc::new(Backends::default()),
             dictionaries: Arc::new(Dictionaries::default()),
-            dictionaries_by_id: Arc::new(DictionariesById::default()),
             config_path: Arc::new(None),
             log_stdout: false,
             log_stderr: false,
@@ -137,22 +134,6 @@ impl ExecuteCtx {
         }
     }
 
-    /// Get the dictionaries for this execution context.
-    pub fn dictionaries_by_id(&self) -> &DictionariesById {
-        &self.dictionaries_by_id
-    }
-
-    /// Set the dictionaries for this execution context.
-    pub fn with_dictionaries_by_id(self, dictionaries: Dictionaries) -> Self {
-        let dicts = dictionaries;
-
-        let dictionaries_by_id = dicts.into_iter().map(|a| (a.1.id, a.1.clone())).collect();
-        Self {
-            dictionaries_by_id: Arc::new(dictionaries_by_id),
-            ..self
-        }
-    }
-
     /// Set the path to the config for this execution context.
     pub fn with_config_path(self, config_path: PathBuf) -> Self {
         Self {
@@ -275,7 +256,6 @@ impl ExecuteCtx {
             remote,
             self.backends.clone(),
             self.dictionaries.clone(),
-            self.dictionaries_by_id.clone(),
             self.config_path.clone(),
         );
         // We currently have to postpone linking and instantiation to the guest task

lib/src/session.rs

@@ -1,21 +1,19 @@
 //! Session type and related facilities.
 
-use crate::config::DictionariesById;
-
 mod body_variant;
 mod downstream;
 
 use {
     self::{body_variant::BodyVariant, downstream::DownstreamResponse},
     crate::{
         body::Body,
-        config::{Backend, Backends, Dictionaries},
+        config::{Backend, Backends, Dictionaries, Dictionary, DictionaryName},
         error::{Error, HandleError},
         logging::LogEndpoint,
         streaming_body::StreamingBody,
         upstream::{PendingRequest, SelectTarget},
         wiggle_abi::types::{
-            BodyHandle, EndpointHandle, PendingRequestHandle, RequestHandle, ResponseHandle,
+            BodyHandle, DictionaryHandle, EndpointHandle, PendingRequestHandle, RequestHandle, ResponseHandle,
         },
     },
     cranelift_entity::PrimaryMap,
@@ -76,7 +74,7 @@ pub struct Session {
     /// The dictionaries configured for this execution.
     ///
     /// Populated prior to guest execution, and never modified.
-    pub(crate) dictionaries_by_id: Arc<DictionariesById>,
+    dictionaries_by_name: PrimaryMap<DictionaryHandle, DictionaryName>,
     /// The path to the configuration file used for this invocation of Viceroy.
     ///
     /// Created prior to guest execution, and never modified.
@@ -96,7 +94,6 @@ impl Session {
         client_ip: IpAddr,
         backends: Arc<Backends>,
         dictionaries: Arc<Dictionaries>,
-        dictionaries_by_id: Arc<DictionariesById>,
         config_path: Arc<Option<PathBuf>>,
     ) -> Session {
         let (parts, body) = req.into_parts();
@@ -108,6 +105,10 @@ impl Session {
         let downstream_req_handle = req_parts.push(Some(parts));
         let downstream_req_body_handle = bodies.push(Some(BodyVariant::Body(body)));
 
+        let mut dictionaries_by_name: PrimaryMap<DictionaryHandle, DictionaryName> = PrimaryMap::new();
+        for name in dictionaries.keys() {
+            dictionaries_by_name.push(name.clone());
+        }
         Session {
             downstream_client_ip: client_ip,
             downstream_req_handle,
@@ -121,7 +122,7 @@ impl Session {
             log_endpoints_by_name: HashMap::new(),
             backends,
             dictionaries,
-            dictionaries_by_id,
+            dictionaries_by_name,
             config_path,
             pending_reqs: PrimaryMap::new(),
             req_id,
@@ -143,7 +144,6 @@ impl Session {
             "0.0.0.0".parse().unwrap(),
             Arc::new(HashMap::new()),
             Arc::new(HashMap::new()),
-            Arc::new(HashMap::new()),
             Arc::new(None),
         )
     }
@@ -509,6 +509,33 @@ impl Session {
     pub fn backend(&self, name: &str) -> Option<&Backend> {
         self.backends.get(name).map(std::ops::Deref::deref)
     }
+
+    // ----- Dictionaries API -----
+
+    /// Look up a dictionary-handle by name.
+    pub fn dictionary_handle(&self, name: &str) -> Result<DictionaryHandle, Error> {
+        let name = DictionaryName::new(name.to_string());
+        for (handle, dictionary_name) in self.dictionaries_by_name.iter() {
+            if &name == dictionary_name {
+                return Ok(handle);
+            }
+        }
+        Err(Error::UnknownDictionary(name))
+        // DictionaryHandle::from(-2)
+    }
+
+    /// Look up a dictionary by dictionary-handle.
+    pub fn dictionary(&self, handle: DictionaryHandle) -> Result<&Dictionary, Error> {
+        match self.dictionaries_by_name.get(handle) {
+            Some(name) => {
+                match self.dictionaries.get(name) {
+                    Some(dictionary) => Ok(dictionary),
+                    None => Err(Error::UnknownDictionaryHandle(handle)),
+                }
+            },
+            None => Err(Error::UnknownDictionaryHandle(handle)),
+        }
+    }
 
     // ----- Pending Requests API -----