Skip to content

Commit

Permalink
replaced HashMap / HashSet with IndexMap / IndexSet (#252)
Browse files Browse the repository at this point in the history
We discussed making this change before but hadn't got around to it.

`IndexMap` / `IndexSet` has a performance benefit and maintains
consistent ordering (makes testing easier).
  • Loading branch information
calvinrp authored Feb 29, 2024
1 parent 7794fa0 commit 7a2d1f5
Show file tree
Hide file tree
Showing 27 changed files with 125 additions and 121 deletions.
38 changes: 20 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ serde_json = "1.0.113"
tokio = { version = "1.36.0", features = ["full"] }
tokio-util = "0.7.10"
serde_with = { version = "3.6.0", features = ["base64"] }
indexmap = { version = "2.2.2", features = ["serde"] }
indexmap = { version = "2.2.4", features = ["serde"] }
tempfile = "3.10.0"
reqwest = { version = "0.11.24", features = ["json", "stream"] }
futures-util = "0.3.30"
Expand Down
4 changes: 2 additions & 2 deletions ci/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! * `./publish verify` - verify crates can be published to crates.io
//! * `./publish publish` - actually publish crates to crates.io
use std::collections::HashMap;
use indexmap::{IndexMap};
use std::env;
use std::fs;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -64,7 +64,7 @@ fn main() {
.iter()
.enumerate()
.map(|(i, c)| (*c, i))
.collect::<HashMap<_, _>>();
.collect::<IndexMap<_, _>>();
crates.sort_by_key(|krate| pos.get(&krate.name[..]));

match &env::args().nth(1).expect("must have one argument")[..] {
Expand Down
1 change: 1 addition & 0 deletions crates/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ serde = { workspace = true }
serde_with = { workspace = true }
thiserror = { workspace = true }
itertools = { workspace = true }
indexmap = { workspace = true }
5 changes: 3 additions & 2 deletions crates/api/src/v1/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
pub use super::ContentSource;
use crate::Status;
use indexmap::IndexMap;
use serde::{de::Unexpected, Deserialize, Serialize, Serializer};
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;
use thiserror::Error;
use warg_crypto::hash::AnyHash;

Expand All @@ -13,7 +14,7 @@ use warg_crypto::hash::AnyHash;
pub struct ContentSourcesResponse {
/// The content sources for the requested content digest, as well as additional
/// content digests imported by the requested content digest.
pub content_sources: HashMap<AnyHash, Vec<ContentSource>>,
pub content_sources: IndexMap<AnyHash, Vec<ContentSource>>,
}

/// Represents a content API error.
Expand Down
13 changes: 7 additions & 6 deletions crates/api/src/v1/fetch.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! Types relating to the fetch API.
use crate::Status;
use indexmap::IndexMap;
use serde::{de::Unexpected, Deserialize, Serialize, Serializer};
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;
use thiserror::Error;
use warg_crypto::hash::AnyHash;
use warg_protocol::{
Expand Down Expand Up @@ -34,8 +35,8 @@ pub struct FetchLogsRequest<'a> {
#[serde(skip_serializing_if = "Option::is_none")]
pub operator: Option<Cow<'a, str>>,
/// The map of package identifiers to last known fetch token.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub packages: Cow<'a, HashMap<LogId, Option<String>>>,
#[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub packages: Cow<'a, IndexMap<LogId, Option<String>>>,
}

/// Represents a fetch logs response.
Expand All @@ -49,8 +50,8 @@ pub struct FetchLogsResponse {
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub operator: Vec<PublishedRecord>,
/// The package records appended since last known package record ids.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub packages: HashMap<LogId, Vec<PublishedRecord>>,
#[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub packages: IndexMap<LogId, Vec<PublishedRecord>>,
}

/// Represents a fetch package names request.
Expand All @@ -68,7 +69,7 @@ pub struct FetchPackageNamesRequest<'a> {
#[serde(rename_all = "camelCase")]
pub struct FetchPackageNamesResponse {
/// The log ID hash mapping to a package name. If `None`, the package name cannot be provided.
pub packages: HashMap<LogId, Option<PackageName>>,
pub packages: IndexMap<LogId, Option<PackageName>>,
}

/// Represents a fetch API error.
Expand Down
13 changes: 7 additions & 6 deletions crates/api/src/v1/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
pub use super::ContentSource;
use crate::Status;
use indexmap::IndexMap;
use serde::{de::Unexpected, Deserialize, Serialize, Serializer};
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;
use thiserror::Error;
use warg_crypto::hash::AnyHash;
use warg_protocol::{
Expand All @@ -25,8 +26,8 @@ pub enum UploadEndpoint {
url: String,
/// Optional header names and values for the upload request.
/// Only `authorization` and `content-type` headers are valid; any other header should be rejected.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
headers: HashMap<String, String>,
#[serde(default, skip_serializing_if = "IndexMap::is_empty")]
headers: IndexMap<String, String>,
},
}

Expand All @@ -50,8 +51,8 @@ pub struct PublishRecordRequest<'a> {
/// The complete set of content sources for the record.
///
/// A registry may not support specifying content sources directly.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub content_sources: HashMap<AnyHash, Vec<ContentSource>>,
#[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub content_sources: IndexMap<AnyHash, Vec<ContentSource>>,
}

/// Represents a package record API entity in a registry.
Expand Down Expand Up @@ -90,7 +91,7 @@ pub enum PackageRecordState {
#[serde(rename_all = "camelCase")]
Sourcing {
/// The digests of the missing content.
missing_content: HashMap<AnyHash, MissingContent>,
missing_content: IndexMap<AnyHash, MissingContent>,
},
/// The package record is processing.
#[serde(rename_all = "camelCase")]
Expand Down
9 changes: 5 additions & 4 deletions crates/client/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
use anyhow::{anyhow, Result};
use bytes::Bytes;
use futures_util::{Stream, TryStreamExt};
use indexmap::IndexMap;
use reqwest::{
header::{HeaderMap, HeaderName, HeaderValue},
Body, IntoUrl, Method, RequestBuilder, Response, StatusCode,
};
use secrecy::{ExposeSecret, Secret};
use serde::de::DeserializeOwned;
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;
use thiserror::Error;
use warg_api::v1::{
content::{ContentError, ContentSourcesResponse},
Expand Down Expand Up @@ -237,8 +238,8 @@ impl Client {
pub async fn latest_checkpoints(
&self,
registries: impl Iterator<Item = &String>,
) -> Result<HashMap<String, SerdeEnvelope<TimestampedCheckpoint>>> {
let mut timestamps = HashMap::new();
) -> Result<IndexMap<String, SerdeEnvelope<TimestampedCheckpoint>>> {
let mut timestamps = IndexMap::new();
for reg in registries.into_iter() {
let url = self.url.join(paths::fetch_checkpoint());
let registry_header = HeaderName::try_from(REGISTRY_HEADER_NAME).unwrap();
Expand Down Expand Up @@ -528,7 +529,7 @@ impl Client {
&self,
method: &str,
url: &str,
headers: &HashMap<String, String>,
headers: &IndexMap<String, String>,
content: impl Into<Body>,
) -> Result<(), ClientError> {
// Upload URLs may be relative to the registry URL.
Expand Down
4 changes: 2 additions & 2 deletions crates/client/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
use crate::{ClientError, RegistryUrl};
use anyhow::{anyhow, Context, Result};
use indexmap::IndexSet;
use normpath::PathExt;
use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
use std::{
collections::HashSet,
env::current_dir,
fs::{self, File},
path::{Component, Path, PathBuf},
Expand Down Expand Up @@ -109,7 +109,7 @@ pub struct Config {

/// List of creds availabe in keyring
#[serde(default, skip_serializing_if = "Option::is_none")]
pub keys: Option<HashSet<String>>,
pub keys: Option<IndexSet<String>>,
}

impl Config {
Expand Down
Loading

0 comments on commit 7a2d1f5

Please sign in to comment.