Merge pull request #857 from holochain/get-links-bug

GetLinks bug (Adds a failing test (with @maackle))

app_spec/build_and_test.sh

@@ -11,6 +11,4 @@ echo "Running test.js in node"
 echo "------------------------------------------------------------------------------------"
 cd test
 npm install
-cd ..
-node test/test.js | test/node_modules/faucet/bin/cmd.js
-node test/regressions.js | test/node_modules/faucet/bin/cmd.js
+npm test

app_spec/test/package.json

@@ -1,13 +1,12 @@
 {
-  "devDependencies": {
-    "faucet": "0.0.1"
-  },
+  "devDependencies": {},
   "dependencies": {
+    "faucet": "0.0.1",
     "json3": "*",
     "sleep": "^5.2.3",
     "tape": "^4.9.1"
   },
   "scripts": {
-    "test": "node test.js"
+    "test": "./run.sh"
   }
 }

app_spec/test/regressions.js

@@ -1,11 +1,12 @@
 
-const sleep = require('sleep');
+const path = require('path')
+const sleep = require('sleep')
 const test = require('tape')
 const { pollFor } = require('./util')
 
 const { Config, Container } = require('../../nodejs_container')
 
-const dnaPath = "./dist/app_spec.hcpkg"
+const dnaPath = path.join(__dirname, "../dist/app_spec.hcpkg")
 const aliceName = "alice"
 const tashName = "tash"
 
@@ -29,11 +30,74 @@ container.start()
 const alice = container.makeCaller(aliceName, dnaPath)
 const tash = container.makeCaller(tashName, dnaPath)
 
+
+// run the following three tests ONE AT A TIME. 
+// The first fails, the second and third pass.
+
+test('calling get_links before link_entries makes a difference', (t) => {
+
+  const get1 = alice.call("blog", "main", "my_posts", {})
+  t.ok(get1.Ok)
+  sleep.sleep(1)
+
+  const create1 = alice.call("blog", "main", "create_post", {content: 'hi'})
+  t.ok(create1.Ok)
+  sleep.sleep(1)
+
+  const get2 = alice.call("blog", "main", "my_posts", {})
+  t.ok(get2.Ok)
+
+  t.equal(get2.Ok.addresses.length, 1)
+  t.end()
+})
+
+
+test('calling get_links twice in a row is different than calling it once', (t) => {
+  // This test is exactly the same as the previous one, but calls my_posts twice in a row.
+  // This makes the links come through the second time.
+
+  const get1 = alice.call("blog", "main", "my_posts", {})
+  t.ok(get1.Ok)
+  sleep.sleep(1)
+
+  const create1 = alice.call("blog", "main", "create_post", {content: 'hi'})
+  t.ok(create1.Ok)
+  sleep.sleep(1)
+
+  alice.call("blog", "main", "my_posts", {})
+  const get2 = alice.call("blog", "main", "my_posts", {})
+  t.ok(get2.Ok)
+
+  t.equal(get2.Ok.addresses.length, 1)
+  t.end()
+})
+
+test('not calling get_links in the beginning helps', (t) => {
+
+  const create1 = alice.call("blog", "main", "create_post", {content: 'hi'})
+  t.ok(create1.Ok)
+  sleep.sleep(1)
+
+  const get1 = alice.call("blog", "main", "my_posts", {})
+  t.ok(get1.Ok)
+
+  t.equal(get1.Ok.addresses.length, 1)
+  t.end()
+})
+
+//////////////////////////////////
+//////////////////////////////////
+
 test('alice create & publish post -> recommend own post to self', async (t) => {
   t.plan(4)
   const content1 = "Holo world...1"
   const in_reply_to = null
   const params = { content: content1, in_reply_to }
+
+
+  // TODO can go away after scenario-api merge
+  const numInitialRecommendedPosts = alice.call('blog', 'main', 'my_recommended_posts', {}).Ok.addresses.length
+
   const postAddr = alice.call("blog", "main", "create_post", params).Ok
   t.ok(postAddr)
 
@@ -49,20 +113,24 @@ test('alice create & publish post -> recommend own post to self', async (t) => {
   console.log("linked: ", linked)
   t.equal(linked.Ok, null)
 
-  sleep.sleep(3)
+  sleep.sleep(1)
 
   const recommendedPosts = alice.call('blog', 'main', 'my_recommended_posts', {})
   console.log("recommendedPosts", recommendedPosts)
   console.log('agent addresses: ', alice.agentId, alice.agentId)
 
-  t.equal(recommendedPosts.Ok.addresses.length, 1)
+  t.equal(recommendedPosts.Ok.addresses.length, numInitialRecommendedPosts + 1)
 })
 
 test('alice create & publish post -> tash recommend to self', async (t) => {
   t.plan(4)
   const content1 = "Holo world...2"
   const in_reply_to = null
   const params = { content: content1, in_reply_to }
+
+  // TODO can go away after scenario-api merge
+  const numInitialRecommendedPosts = tash.call('blog', 'main', 'my_recommended_posts', {}).Ok.addresses.length
+
   const postAddr = alice.call("blog", "main", "create_post", params).Ok
   t.ok(postAddr)
 
@@ -78,20 +146,24 @@ test('alice create & publish post -> tash recommend to self', async (t) => {
   console.log("linked: ", linked)
   t.equal(linked.Ok, null)
 
-  sleep.sleep(3)
+  sleep.sleep(1)
 
   const recommendedPosts = tash.call('blog', 'main', 'my_recommended_posts', {})
   console.log("recommendedPosts", recommendedPosts)
   console.log('agent addresses: ', alice.agentId, tash.agentId)
 
-  t.equal(recommendedPosts.Ok.addresses.length, 1)
+  t.equal(recommendedPosts.Ok.addresses.length, numInitialRecommendedPosts + 1)
 })
 
 test('create & publish post -> recommend to other agent', async (t) => {
   t.plan(4)
   const content1 = "Holo world...3"
   const in_reply_to = null
   const params = { content: content1, in_reply_to }
+
+  // TODO can go away after scenario-api merge
+  const numInitialRecommendedPosts = tash.call('blog', 'main', 'my_recommended_posts', {}).Ok.addresses.length
+
   const postAddr = alice.call("blog", "main", "create_post", params).Ok
   t.ok(postAddr)
 
@@ -107,11 +179,11 @@ test('create & publish post -> recommend to other agent', async (t) => {
   console.log("linked: ", linked)
   t.equal(linked.Ok, null)
 
-  sleep.sleep(3)
+  sleep.sleep(1)
 
   const recommendedPosts = tash.call('blog', 'main', 'my_recommended_posts', {})
   console.log("recommendedPosts", recommendedPosts)
   console.log('agent addresses: ', alice.agentId, tash.agentId)
 
-  t.equal(recommendedPosts.Ok.addresses.length, 1)
+  t.equal(recommendedPosts.Ok.addresses.length, numInitialRecommendedPosts + 1)
 })

app_spec/test/run.sh

@@ -0,0 +1,6 @@
+if [ -z $1 ] 
+then
+	faucet test.js regressions.js
+else
+	faucet $1
+fi

app_spec/test/test.js

@@ -1,11 +1,12 @@
 
-const sleep = require('sleep');
+const path = require('path')
+const sleep = require('sleep')
 const test = require('tape')
 const { pollFor } = require('./util')
 
 const { Config, Container } = require('../../nodejs_container')
 
-const dnaPath = "./dist/app_spec.hcpkg"
+const dnaPath = path.join(__dirname, "../dist/app_spec.hcpkg")
 const aliceName = "alice"
 const tashName = "tash"
 
@@ -29,6 +30,7 @@ container.start()
 const alice = container.makeCaller(aliceName, dnaPath)
 const tash = container.makeCaller(tashName, dnaPath)
 
+
 test('agentId', (t) => {
   t.plan(2)
   t.ok(alice.agentId)

core/src/action.rs

@@ -111,7 +111,7 @@ pub enum Action {
     Publish(Address),
 
     /// GetEntry by address
-    GetEntry(Address),
+    GetEntry(GetEntryKey),
 
     /// Lets the network module respond to a GET request.
     /// Triggered from the corresponding workflow after retrieving the
@@ -128,11 +128,12 @@ pub enum Action {
     ///
     RemoveEntry((Address, Address)),
     ///
-    GetEntryTimeout(Address),
+    GetEntryTimeout(GetEntryKey),
 
     /// get links from entry address and tag name
-    GetLinks((Address, String)),
-    GetLinksTimeout((Address, String)),
+    /// Last string is the stringified process unique id of this `hdk::get_links` call.
+    GetLinks(GetLinksKey),
+    GetLinksTimeout(GetLinksKey),
     RespondGetLinks((GetDhtMetaData, Vec<Address>)),
     HandleGetLinksResult((DhtMetaData, String)),
 
@@ -206,6 +207,31 @@ pub type NetworkReduceFn = ReduceFn<NetworkState>;
 pub type NucleusReduceFn = ReduceFn<NucleusState>;
 pub type ReduceFn<S> = fn(Arc<Context>, &mut S, &ActionWrapper);
 
+/// The unique key that represents a GetLinks request, used to associate the eventual
+/// response with this GetLinks request
+#[derive(Clone, PartialEq, Eq, Hash, Debug)]
+pub struct GetLinksKey {
+    /// The address of the Link base
+    pub base_address: Address,
+
+    /// The link tag
+    pub tag: String,
+
+    /// A unique ID that is used to pair the eventual result to this request
+    pub id: String,
+}
+
+/// The unique key that represents a Get request, used to associate the eventual
+/// response with this Get request
+#[derive(Clone, PartialEq, Eq, Hash, Debug)]
+pub struct GetEntryKey {
+    /// The address of the entry to get
+    pub address: Address,
+
+    /// A unique ID that is used to pair the eventual result to this request
+    pub id: String,
+}
+
 /// Everything the network module needs to know in order to send a
 /// direct message.
 #[derive(Clone, PartialEq, Debug)]
@@ -245,15 +271,18 @@ pub struct NetworkSettings {
 pub mod tests {
 
     use crate::{
-        action::{Action, ActionWrapper},
+        action::{Action, ActionWrapper, GetEntryKey},
         nucleus::tests::test_call_response,
     };
     use holochain_core_types::entry::{expected_entry_address, test_entry};
     use test_utils::calculate_hash;
 
     /// dummy action
     pub fn test_action() -> Action {
-        Action::GetEntry(expected_entry_address())
+        Action::GetEntry(GetEntryKey {
+            address: expected_entry_address(),
+            id: String::from("test-id"),
+        })
     }
 
     /// dummy action wrapper with test_action()
@@ -268,7 +297,10 @@ pub mod tests {
 
     /// dummy action for a get of test_hash()
     pub fn test_action_wrapper_get() -> ActionWrapper {
-        ActionWrapper::new(Action::GetEntry(expected_entry_address()))
+        ActionWrapper::new(Action::GetEntry(GetEntryKey {
+            address: expected_entry_address(),
+            id: snowflake::ProcessUniqueId::new().to_string(),
+        }))
     }
 
     pub fn test_action_wrapper_rzfr() -> ActionWrapper {

core/src/network/actions/get_entry.rs

@@ -1,6 +1,6 @@
 extern crate futures;
 use crate::{
-    action::{Action, ActionWrapper},
+    action::{Action, ActionWrapper, GetEntryKey},
     context::Context,
     instance::dispatch_action,
 };
@@ -20,26 +20,31 @@ pub async fn get_entry<'a>(
     context: &'a Arc<Context>,
     address: &'a Address,
 ) -> HcResult<Option<EntryWithMeta>> {
-    let action_wrapper = ActionWrapper::new(Action::GetEntry(address.clone()));
+    let key = GetEntryKey {
+        address: address.clone(),
+        id: snowflake::ProcessUniqueId::new().to_string(),
+    };
+
+    let action_wrapper = ActionWrapper::new(Action::GetEntry(key.clone()));
     dispatch_action(context.action_channel(), action_wrapper.clone());
 
     let _ = async {
         sleep(Duration::from_secs(60));
-        let action_wrapper = ActionWrapper::new(Action::GetEntryTimeout(address.clone()));
+        let action_wrapper = ActionWrapper::new(Action::GetEntryTimeout(key.clone()));
         dispatch_action(context.action_channel(), action_wrapper.clone());
     };
 
     await!(GetEntryFuture {
         context: context.clone(),
-        address: address.clone(),
+        key
     })
 }
 
 /// GetEntryFuture resolves to a HcResult<Entry>.
 /// Tracks the state of the network module
 pub struct GetEntryFuture {
     context: Arc<Context>,
-    address: Address,
+    key: GetEntryKey,
 }
 
 impl Future for GetEntryFuture {
@@ -55,7 +60,7 @@ impl Future for GetEntryFuture {
         // See: https://github.com/holochain/holochain-rust/issues/314
         //
         lw.wake();
-        match state.get_entry_with_meta_results.get(&self.address) {
+        match state.get_entry_with_meta_results.get(&self.key) {
             Some(Some(result)) => Poll::Ready(result.clone()),
             _ => Poll::Pending,
         }