GH-633: review five

multinode_integration_tests/src/masq_mock_node.rs

@@ -6,7 +6,6 @@ use crate::masq_node::MASQNodeUtils;
 use crate::masq_node::NodeReference;
 use crate::masq_node::PortSelector;
 use crate::multinode_gossip::{Introduction, MultinodeGossip, SingleNode};
-use itertools::Either;
 use masq_lib::blockchains::chains::Chain;
 use masq_lib::test_utils::utils::TEST_DEFAULT_MULTINODE_CHAIN;
 use node_lib::hopper::live_cores_package::LiveCoresPackage;
@@ -43,8 +42,10 @@ use std::time::{Duration, Instant};
 
 pub struct MASQMockNode {
     control_stream: RefCell<TcpStream>,
-    //keep this smart pointer because any drop() called on a clone of the naked structure
-    //would stop the Docker container for this Node...or redesign...
+    // retain this Rc pointer because as long as there is at least one reference we won't drop
+    // the actual structure, instead, only the reference count will be affected; unlike to situation
+    // with this structure creating its clones directly, then the whole Docker container would
+    // immediately halt for this Node...because that's how its Drop implementation works...
     guts: Rc<MASQMockNodeGuts>,
 }
 
@@ -146,102 +147,85 @@ impl MASQNode for MASQMockNode {
     }
 }
 
-pub struct MASQMockNodeWithMutableHandle {
+pub struct MutableMASQMockNode {
     pub control_stream: RefCell<TcpStream>,
     configurable_guts: MASQMockNodeGuts,
 }
 
-impl MASQMockNodeWithMutableHandle {
+impl MutableMASQMockNode {
     pub fn absorb_configuration(&mut self, node_record: &NodeRecord) {
         // Copy attributes from the NodeRecord into the MASQNode.
         self.configurable_guts.earning_wallet = node_record.earning_wallet();
         self.configurable_guts.rate_pack = *node_record.rate_pack();
     }
+}
 
-    pub fn finalize_mutable_handle(self) -> MASQMockNode {
+impl From<MutableMASQMockNode> for MASQMockNode {
+    fn from(mutable_handle: MutableMASQMockNode) -> Self {
         MASQMockNode {
-            control_stream: self.control_stream,
-            guts: Rc::new(self.configurable_guts),
+            control_stream: mutable_handle.control_stream,
+            guts: Rc::new(mutable_handle.configurable_guts),
         }
     }
 }
 
-pub enum MASQMockNodeStarterMode {
-    Direct,
-    WithTemporarilyMutableHandle,
+pub trait MASQMockNodeStarter<T> {
+    fn start(
+        &self,
+        ports: Vec<u16>,
+        index: usize,
+        host_node_parent_dir: Option<String>,
+        public_key_opt: Option<&PublicKey>,
+        chain: Chain,
+    ) -> T;
 }
 
-pub struct MASQMockNodeStarter {}
+pub struct ImmutableMASQMockNodeStarter {}
 
-impl MASQMockNodeStarter {
-    pub fn start(
+impl MASQMockNodeStarter<MASQMockNode> for ImmutableMASQMockNodeStarter {
+    fn start(
+        &self,
         ports: Vec<u16>,
         index: usize,
         host_node_parent_dir: Option<String>,
-        starter_mode: MASQMockNodeStarterMode,
         public_key_opt: Option<&PublicKey>,
         chain: Chain,
-    ) -> Either<MASQMockNode, MASQMockNodeWithMutableHandle> {
-        let cryptde_enum = Self::initiate_cryptde_enum(public_key_opt, chain);
-        Self::start_with_cryptde_enum(
+    ) -> MASQMockNode {
+        let (control_stream, mock_node_guts) = MASQMockNode::start_masq_mock_nodes_with_bare_guts(
             ports,
             index,
-            starter_mode,
             host_node_parent_dir,
-            cryptde_enum,
-        )
+            public_key_opt,
+            chain,
+        );
+        MASQMockNode {
+            control_stream,
+            guts: Rc::new(mock_node_guts),
+        }
     }
+}
 
-    fn start_with_cryptde_enum(
+pub struct MutableMASQMockNodeStarter {}
+
+impl MASQMockNodeStarter<MutableMASQMockNode> for MutableMASQMockNodeStarter {
+    fn start(
+        &self,
         ports: Vec<u16>,
         index: usize,
-        starter_mode: MASQMockNodeStarterMode,
         host_node_parent_dir: Option<String>,
-        cryptde_enum: CryptDEEnum,
-    ) -> Either<MASQMockNode, MASQMockNodeWithMutableHandle> {
-        let name = format!("mock_node_{}", index);
-        let node_addr = NodeAddr::new(&IpAddr::V4(Ipv4Addr::new(172, 18, 1, index as u8)), &ports);
-        let earning_wallet = make_wallet(format!("{}_earning", name).as_str());
-        let consuming_wallet = Some(make_paying_wallet(format!("{}_consuming", name).as_bytes()));
-        MASQNodeUtils::clean_up_existing_container(&name[..]);
-        MASQMockNode::do_docker_run(&node_addr, host_node_parent_dir, &name);
-        let wait_addr = SocketAddr::new(node_addr.ip_addr(), CONTROL_STREAM_PORT);
-        let control_stream = RefCell::new(MASQMockNode::wait_for_startup(wait_addr, &name));
-        let framer = RefCell::new(DataHunkFramer::new());
-        let guts = MASQMockNodeGuts {
-            name,
-            node_addr,
-            earning_wallet,
-            consuming_wallet,
-            rate_pack: DEFAULT_RATE_PACK,
-            cryptde_enum,
-            framer,
-            chain: TEST_DEFAULT_MULTINODE_CHAIN,
-        };
-        match starter_mode {
-            MASQMockNodeStarterMode::Direct => Either::Left(MASQMockNode {
-                control_stream,
-                guts: Rc::new(guts),
-            }),
-            MASQMockNodeStarterMode::WithTemporarilyMutableHandle => {
-                Either::Right(MASQMockNodeWithMutableHandle {
-                    control_stream,
-                    configurable_guts: guts,
-                })
-            }
-        }
-    }
-
-    fn initiate_cryptde_enum(public_key_opt: Option<&PublicKey>, chain: Chain) -> CryptDEEnum {
-        match public_key_opt {
-            Some(public_key) => {
-                let main_cryptde = CryptDENull::from(public_key, chain);
-                let mut key = public_key.as_slice().to_vec();
-                key.reverse();
-                let alias_cryptde = CryptDENull::from(&PublicKey::new(&key), chain);
-                CryptDEEnum::Fake((main_cryptde, alias_cryptde))
-            }
-            None => CryptDEEnum::Real(CryptDEReal::new(chain)),
+        public_key_opt: Option<&PublicKey>,
+        chain: Chain,
+    ) -> MutableMASQMockNode {
+        let (control_stream, mock_node_guts) = MASQMockNode::start_masq_mock_nodes_with_bare_guts(
+            ports,
+            index,
+            host_node_parent_dir,
+            public_key_opt,
+            chain,
+        );
+        MutableMASQMockNode {
+            control_stream,
+            configurable_guts: mock_node_guts,
         }
     }
 }
@@ -377,12 +361,7 @@ impl MASQMockNode {
     ) -> Result<(SocketAddr, SocketAddr, LiveCoresPackage), Error> {
         let stop_at = Instant::now().add(timeout);
         let mut accumulated_data: Vec<u8> = vec![];
-        // dunno why these are a problem; they _are_ used on the last line of the function.
-        #[allow(unused_assignments)]
-        let mut from_opt: Option<SocketAddr> = None;
-        #[allow(unused_assignments)]
-        let mut to_opt: Option<SocketAddr> = None;
-        let unmasked_chunk: Vec<u8> = loop {
+        let (unmasked_chunk, socket_from, socket_to) = loop {
             match self.wait_for_data(Duration::from_millis(100)) {
                 Err(ref e) if e.kind() == ErrorKind::WouldBlock => {
                     if Instant::now() > stop_at {
@@ -393,14 +372,14 @@ impl MASQMockNode {
                 Err(e) => return Err(e),
                 Ok(data_hunk) => {
                     accumulated_data.extend(data_hunk.data);
-                    from_opt = Some(data_hunk.from);
-                    to_opt = Some(data_hunk.to);
                     match masquerader.try_unmask(&accumulated_data) {
                         Err(MasqueradeError::NotThisMasquerader) => {
                             panic!("Wrong Masquerader supplied to wait_for_package")
                         }
                         Err(_) => continue,
-                        Ok(unmasked_chunk) => break unmasked_chunk.chunk,
+                        Ok(unmasked_chunk) => {
+                            break (unmasked_chunk.chunk, data_hunk.from, data_hunk.to)
+                        }
                     }
                 }
             }
@@ -412,7 +391,7 @@ impl MASQMockNode {
             .unwrap();
         let live_cores_package =
             serde_cbor::de::from_slice::<LiveCoresPackage>(decrypted_data.as_slice()).unwrap();
-        Ok((from_opt.unwrap(), to_opt.unwrap(), live_cores_package))
+        Ok((socket_from, socket_to, live_cores_package))
     }
 
     pub fn wait_for_gossip(&self, timeout: Duration) -> Option<(Gossip_0v1, IpAddr)> {
@@ -472,6 +451,58 @@ impl MASQMockNode {
         stream.shutdown(Shutdown::Both).unwrap();
     }
 
+    fn start_masq_mock_nodes_with_bare_guts(
+        ports: Vec<u16>,
+        index: usize,
+        host_node_parent_dir: Option<String>,
+        public_key_opt: Option<&PublicKey>,
+        chain: Chain,
+    ) -> (RefCell<TcpStream>, MASQMockNodeGuts) {
+        let cryptde_enum = Self::initiate_cryptde_enum(public_key_opt, chain);
+        Self::start_with_cryptde_enum(ports, index, host_node_parent_dir, cryptde_enum)
+    }
+
+    fn initiate_cryptde_enum(public_key_opt: Option<&PublicKey>, chain: Chain) -> CryptDEEnum {
+        match public_key_opt {
+            Some(public_key) => {
+                let main_cryptde = CryptDENull::from(public_key, chain);
+                let mut key = public_key.as_slice().to_vec();
+                key.reverse();
+                let alias_cryptde = CryptDENull::from(&PublicKey::new(&key), chain);
+                CryptDEEnum::Fake((main_cryptde, alias_cryptde))
+            }
+            None => CryptDEEnum::Real(CryptDEReal::new(chain)),
+        }
+    }
+
+    fn start_with_cryptde_enum(
+        ports: Vec<u16>,
+        index: usize,
+        host_node_parent_dir: Option<String>,
+        cryptde_enum: CryptDEEnum,
+    ) -> (RefCell<TcpStream>, MASQMockNodeGuts) {
+        let name = format!("mock_node_{}", index);
+        let node_addr = NodeAddr::new(&IpAddr::V4(Ipv4Addr::new(172, 18, 1, index as u8)), &ports);
+        let earning_wallet = make_wallet(format!("{}_earning", name).as_str());
+        let consuming_wallet = Some(make_paying_wallet(format!("{}_consuming", name).as_bytes()));
+        MASQNodeUtils::clean_up_existing_container(&name[..]);
+        MASQMockNode::do_docker_run(&node_addr, host_node_parent_dir, &name);
+        let wait_addr = SocketAddr::new(node_addr.ip_addr(), CONTROL_STREAM_PORT);
+        let control_stream = RefCell::new(MASQMockNode::wait_for_startup(wait_addr, &name));
+        let framer = RefCell::new(DataHunkFramer::new());
+        let guts = MASQMockNodeGuts {
+            name,
+            node_addr,
+            earning_wallet,
+            consuming_wallet,
+            rate_pack: DEFAULT_RATE_PACK,
+            cryptde_enum,
+            framer,
+            chain: TEST_DEFAULT_MULTINODE_CHAIN,
+        };
+        (control_stream, guts)
+    }
+
     fn do_docker_run(node_addr: &NodeAddr, host_node_parent_dir: Option<String>, name: &str) {
         let root = match host_node_parent_dir {
             Some(dir) => dir,

multinode_integration_tests/src/masq_node_cluster.rs

@@ -1,12 +1,12 @@
 // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved.
 use crate::command::Command;
 use crate::masq_mock_node::{
-    MASQMockNode, MASQMockNodeStarter, MASQMockNodeStarterMode, MASQMockNodeWithMutableHandle,
+    ImmutableMASQMockNodeStarter, MASQMockNode, MASQMockNodeStarter, MutableMASQMockNode,
+    MutableMASQMockNodeStarter,
 };
 use crate::masq_node::{MASQNode, MASQNodeUtils};
 use crate::masq_real_node::MASQRealNode;
 use crate::masq_real_node::NodeStartupConfig;
-use itertools::Either;
 use masq_lib::blockchains::chains::Chain;
 use masq_lib::test_utils::utils::TEST_DEFAULT_MULTINODE_CHAIN;
 use node_lib::sub_lib::cryptde::PublicKey;
@@ -91,63 +91,45 @@ impl MASQNodeCluster {
         self.start_mock_node_added_to_cluster(ports, Some(public_key))
     }
 
-    pub fn start_configurable_mock_node_with_public_key(
+    pub fn start_mutable_mock_node_with_public_key(
         &mut self,
         ports: Vec<u16>,
         public_key: &PublicKey,
-    ) -> MASQMockNodeWithMutableHandle {
-        match self.start_mock_node(
-            ports,
-            MASQMockNodeStarterMode::WithTemporarilyMutableHandle,
-            Some(public_key),
-        ) {
-            Either::Right(mock_node_mutable_handle) => mock_node_mutable_handle,
-            _ => panic!(
-                "This is expected to return MASQMockNodeMutablePrototype but we hit MASQMockNode"
-            ),
-        }
+    ) -> MutableMASQMockNode {
+        self.start_mock_node(&MutableMASQMockNodeStarter {}, ports, Some(public_key))
     }
 
     fn start_mock_node_added_to_cluster(
         &mut self,
         ports: Vec<u16>,
         public_key_opt: Option<&PublicKey>,
     ) -> MASQMockNode {
-        match self.start_mock_node(ports, MASQMockNodeStarterMode::Direct, public_key_opt) {
-            Either::Left(masq_mock_node) => {
-                let name = masq_mock_node.name().to_string();
-                self.mock_nodes.insert(name.clone(), masq_mock_node);
-                self.mock_nodes.get(&name).unwrap().clone()
-            }
-            _ => panic!(
-                "This is expected to return MASQMockNode but we hit MASQMockNodeMutablePrototype"
-            ),
-        }
+        let mock_node =
+            self.start_mock_node(&ImmutableMASQMockNodeStarter {}, ports, public_key_opt);
+        let name = mock_node.name().to_string();
+        self.mock_nodes.insert(name.clone(), mock_node);
+        self.mock_nodes.get(&name).unwrap().clone()
     }
 
-    fn start_mock_node(
+    fn start_mock_node<T>(
         &mut self,
+        mock_node_starter: &dyn MASQMockNodeStarter<T>,
         ports: Vec<u16>,
-        mock_node_starter_mode: MASQMockNodeStarterMode,
         public_key_opt: Option<&PublicKey>,
-    ) -> Either<MASQMockNode, MASQMockNodeWithMutableHandle> {
+    ) -> T {
         let index = self.next_index;
         self.next_index += 1;
-        MASQMockNodeStarter::start(
+        mock_node_starter.start(
             ports,
             index,
             self.host_node_parent_dir.clone(),
-            mock_node_starter_mode,
             public_key_opt,
             self.chain,
         )
     }
 
-    pub fn finalize_mock_node_with_mutable_handle(
-        &mut self,
-        mutable_mock_node_handle: MASQMockNodeWithMutableHandle,
-    ) -> MASQMockNode {
-        let mock_node = mutable_mock_node_handle.finalize_mutable_handle();
+    pub fn finalize_and_add(&mut self, mutable_mock_node: MutableMASQMockNode) -> MASQMockNode {
+        let mock_node = MASQMockNode::from(mutable_mock_node);
         let name = mock_node.name().to_string();
         self.mock_nodes.insert(name.clone(), mock_node);
         self.mock_nodes.get(&name).unwrap().clone()

multinode_integration_tests/src/neighborhood_constructor.rs

@@ -188,9 +188,9 @@ fn form_mock_node_skeleton(
         .into_iter()
         .map(|model_node_key| {
             let mut configurable_node =
-                cluster.start_configurable_mock_node_with_public_key(vec![10000], model_node_key);
+                cluster.start_mutable_mock_node_with_public_key(vec![10000], model_node_key);
             configurable_node.absorb_configuration(model_db.node_by_key(model_node_key).unwrap());
-            let node = cluster.finalize_mock_node_with_mutable_handle(configurable_node);
+            let node = cluster.finalize_and_add(configurable_node);
             node.transmit_debut(real_node).unwrap();
             node.wait_for_gossip(Duration::from_secs(2)).unwrap();
             let standard_gossip = StandardBuilder::new()

multinode_integration_tests/tests/data_routing_test.rs

@@ -316,7 +316,7 @@ fn multiple_stream_zero_hop_test() {
     let mut another_client = zero_hop_node.make_client(8080);
 
     one_client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n");
-    another_client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.neverssl.com\r\n\r\n");
+    another_client.send_chunk(b"GET /online/ HTTP/1.1\r\nHost: whatever.neverssl.com\r\n\r\n");
 
     let one_response = one_client.wait_for_chunk();
     let another_response = another_client.wait_for_chunk();
@@ -328,7 +328,11 @@ fn multiple_stream_zero_hop_test() {
         String::from_utf8(one_response).unwrap()
     );
     assert_eq!(
-        index_of(&another_response, &b"NeverSSL - Connecting ..."[..],).is_some(),
+        index_of(
+            &another_response,
+            &b"neverssl.com will never use SSL (also known as TLS)"[..],
+        )
+        .is_some(),
         true,
         "Actual response:\n{}",
         String::from_utf8(another_response).unwrap()