From 38a92229d7bdccf4b8b414e230cf2ba98cb2b297 Mon Sep 17 00:00:00 2001
From: Nick Alexander <nalexander@mozilla.com>
Date: Thu, 12 Jul 2018 16:50:08 -0700
Subject: [PATCH] Pre: Replace `PartitionMapping` trait with newtype. r=grisha

Generally, I think that Mentat is using too many small traits rather
than wrapping types into newtypes.  Wrapping into newtypes is cheap in
Rust, and it makes it easier to reason about the code.
---
 db/src/db.rs    | 20 ++++++++------------
 db/src/tx.rs    |  1 -
 db/src/types.rs | 34 +++++++++++++++++++++++++++++++---
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/db/src/db.rs b/db/src/db.rs
index e52a7384..dfffe9f9 100644
--- a/db/src/db.rs
+++ b/db/src/db.rs
@@ -14,12 +14,10 @@ use failure::{
     ResultExt,
 };
 
-use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::collections::hash_map::{
     Entry,
 };
-use std::fmt::Display;
 use std::iter::{once, repeat};
 use std::ops::Range;
 use std::path::Path;
@@ -1080,20 +1078,14 @@ SELECT EXISTS
     Ok(())
 }
 
-pub trait PartitionMapping {
-    fn allocate_entid<S: ?Sized + Ord + Display>(&mut self, partition: &S) -> i64 where String: Borrow<S>;
-    fn allocate_entids<S: ?Sized + Ord + Display>(&mut self, partition: &S, n: usize) -> Range<i64> where String: Borrow<S>;
-    fn contains_entid(&self, entid: Entid) -> bool;
-}
-
-impl PartitionMapping for PartitionMap {
+impl PartitionMap {
     /// Allocate a single fresh entid in the given `partition`.
-    fn allocate_entid<S: ?Sized + Ord + Display>(&mut self, partition: &S) -> i64 where String: Borrow<S> {
+    pub(crate) fn allocate_entid(&mut self, partition: &str) -> i64 {
         self.allocate_entids(partition, 1).start
     }
 
     /// Allocate `n` fresh entids in the given `partition`.
-    fn allocate_entids<S: ?Sized + Ord + Display>(&mut self, partition: &S, n: usize) -> Range<i64> where String: Borrow<S> {
+    pub(crate) fn allocate_entids(&mut self, partition: &str, n: usize) -> Range<i64> {
         match self.get_mut(partition) {
             Some(partition) => {
                 let idx = partition.index;
@@ -1105,7 +1097,7 @@ impl PartitionMapping for PartitionMap {
         }
     }
 
-    fn contains_entid(&self, entid: Entid) -> bool {
+    pub(crate) fn contains_entid(&self, entid: Entid) -> bool {
         self.values().any(|partition| partition.contains_entid(entid))
     }
 }
@@ -1114,6 +1106,10 @@ impl PartitionMapping for PartitionMap {
 mod tests {
     extern crate env_logger;
 
+    use std::borrow::{
+        Borrow,
+    };
+
     use super::*;
     use debug::{TestConn,tempids};
     use edn::{
diff --git a/db/src/tx.rs b/db/src/tx.rs
index 4179fa24..85010f4b 100644
--- a/db/src/tx.rs
+++ b/db/src/tx.rs
@@ -60,7 +60,6 @@ use std::iter::{
 use db;
 use db::{
     MentatStoring,
-    PartitionMapping,
 };
 use edn::{
     InternSet,
diff --git a/db/src/types.rs b/db/src/types.rs
index 8d951e54..9416dcbe 100644
--- a/db/src/types.rs
+++ b/db/src/types.rs
@@ -10,10 +10,17 @@
 
 #![allow(dead_code)]
 
-use std::collections::HashMap;
 use std::collections::{
     BTreeMap,
     BTreeSet,
+    HashMap,
+};
+use std::iter::{
+    FromIterator,
+};
+use std::ops::{
+    Deref,
+    DerefMut,
 };
 
 extern crate mentat_core;
@@ -37,7 +44,7 @@ use edn::entities::{
 use errors;
 
 /// Represents one partition of the entid space.
-#[derive(Clone,Debug,Eq,Hash,Ord,PartialOrd,PartialEq)]
+#[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)]
 #[cfg_attr(feature = "syncable", derive(Serialize,Deserialize))]
 pub struct Partition {
     /// The first entid in the partition.
@@ -66,7 +73,28 @@ impl Partition {
 }
 
 /// Map partition names to `Partition` instances.
-pub type PartitionMap = BTreeMap<String, Partition>;
+#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialOrd, PartialEq)]
+pub struct PartitionMap(BTreeMap<String, Partition>);
+
+impl Deref for PartitionMap {
+    type Target = BTreeMap<String, Partition>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl DerefMut for PartitionMap {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl FromIterator<(String, Partition)> for PartitionMap {
+    fn from_iter<T: IntoIterator<Item=(String, Partition)>>(iter: T) -> Self {
+        PartitionMap(iter.into_iter().collect())
+    }
+}
 
 /// Represents the metadata required to query from, or apply transactions to, a Mentat store.
 ///