Add dynamic link tagging #1402

willemolding · 2019-05-14T06:52:23Z

PR summary

Depends on #1412

This PR adds tags to links with the following properties:

All links must have a tag
The tag of a link will be retrieved when hdk::get_links is called
It is possible to only get links that exactly match a given tag
It is possible to retrieve all links irrespective of the tag (and it will return all the tags)

Works great except introduced some flaky tests which need sorting out before merging. It is ready for review though

…al tag

core/src/action.rs

willemolding · 2019-05-14T06:54:23Z

core/src/dht/dht_store.rs

-            ]),
+            EavFilter::predicate(move |attr| {
+                match attr {
+                    Attribute::LinkTag(query_link_type, query_tag) | Attribute::RemovedLink(query_link_type, query_tag) => {


Highlight: This is where filtering is done based on if a tag is provided (so match both tag and type) or only a type is provided (return all tags of that type)

core/src/workflows/get_link_result.rs

core/src/nucleus/ribosome/api/get_links.rs

…oring index information about a given entry. This example uses topics which posts can be associated to and indexed.

… option.

… once hdk issue is sorted - this should assert for match of post

lucksus · 2019-05-15T16:39:04Z

core/src/network/actions/get_links.rs

@@ -17,11 +17,13 @@ use std::{pin::Pin, sync::Arc, thread};
 pub async fn get_links(
    context: Arc<Context>,
    address: Address,
-    tag: String,
+    link_type: String,


Hm, I guess we also want this to be an Option

Suggested change

link_type: String,

link_type: Option<String>,

so that we could requests all links with the same tag but different types. That would match the get_links semantic we had in proto, no?
@zippy

So you think we should make link_type optional as well? I guess this is possible but not sure if it makes sense...

I am also not sure if we should be forcing every link to be tagged. This is how it works currently but I suppose we could let people pass a None to link_entries and have the HDK add a sensible default behind the scenes. There are a few usability things to sort out.

I guess it makes sense to have no tag, i.e. None.

It does make sense to have the type optional in the get_links signature so that we can requests all links with the same tag but from different types.

Imagine a use-case where agents can add all different kinds of comments on entries, each being a link with a tag comment. But the reason why that comment is a valid statement could differ which would best be modeled by having multiple link types with different validation rules. But for rendering all those comments a UI might not care about why something is a valid comment, not caring for the type but instead requesting any link with the tag comment independent of the type.

… t.ok assertion.

lucksus

This is great!

But: the changelog is off and I would really love to see a test that shows getting links with a given tag works as expected (when there are multiple links with different tags).

CHANGELOG-UNRELEASED.md

willemolding · 2019-05-21T02:25:21Z

Added the ability to retrieve based on type, tag, type+tag or everything.

Also added a test for getting the exact match tag (then there are other links to filter out) in the app spec and rust tests

lucksus · 2019-05-21T12:40:47Z

CHANGELOG-UNRELEASED.md

@@ -6,12 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ### Added

- Option to show NPM output when pulling deps during `hc test` [PR#1401](https://github.com/holochain/holochain-rust/pull/1401)
- Adds scaffolding/skeleton for a future WASM conductor [#894](https://github.com/holochain/holochain-rust/pull/894)


These lines must stay in! Changelogs from other PRs that got merged before!

I'll get it right one day 🤦‍♂️

lucksus

Ok, I have put back the changelog items you removed by mistake, see 40f6df8.

Looks great now!

zippy

Looks great. Marking approved, though needs a few updates to docs and comments!

CHANGELOG-UNRELEASED.md

core_types/src/link/mod.rs

core/src/network/state.rs

doc/holochain_101/src/first_steps.md

doc/holochain_101/src/zome/api_functions.md

hdk-rust/src/api/get_links.rs

hdk-rust/src/api/link_entries.rs

Co-Authored-By: Eric Harris-Braun <[email protected]>

…ain/holochain-rust into add-dynamic-link-tag-core

pdaoust · 2019-05-23T22:46:46Z

wasm_utils/src/api_serialization/get_links.rs

@@ -41,6 +42,7 @@ impl Default for GetLinksOptions {
 pub struct LinksResult {
    pub address: Address,
    pub headers: Vec<ChainHeader>,
+    pub tag: String,


I realise I'm late to the game here, but now that the link_type param in get_links is an Option, should LinksResult also include the link type?

willemolding added 13 commits May 13, 2019 16:58

makes changes to back-end references. Still some to go

4119509

test passing with changes amde

6582282

updates app specv

e65133c

fmt

a106a9e

changelog

8345f49

adds link tag field but not used by eav or hdk

23c5c72

adds test that different tagged links have a different hash

487781e

adds test for dedup and unique tag retrieval of links

eddb65d

something get links

3329e86

changes how links are handled to include a tag

b916b9b

adds required tag field to GetLinksArgs. tests passing

e1e9766

makes EAV filter for links more verbose to enable filtering on option…

4c45220

…al tag

adds dummy return value for tag on LinkResult

294a9c7

willemolding added the WIP work in progress label May 14, 2019

willemolding commented May 14, 2019

View reviewed changes

core/src/action.rs Outdated Show resolved Hide resolved

willemolding commented May 14, 2019

View reviewed changes

core/src/workflows/get_link_result.rs Outdated Show resolved Hide resolved

willemolding commented May 14, 2019

View reviewed changes

core/src/nucleus/ribosome/api/get_links.rs Show resolved Hide resolved

willemolding and others added 10 commits May 15, 2019 08:14

makes tag optional in GetLinkKey for wildcard requests

7973bdd

returns request tag rather than no tag from get_links_workflow

2f08840

updates tests to reflect expected behaviour of returned tags

99f548d

Merge branch 'develop' into add-dynamic-link-tag-core

72a9d16

fmt

fe2a23d

merge

46fbfb7

Added new scenarios in app_spec which showcase using link tags for st…

ab6d48e

…oring index information about a given entry. This example uses topics which posts can be associated to and indexed.

Changed hdk crate functions: link_entries and get_links to accept tag…

a966645

… option.

fmt

7d9accf

query result assertions now check for ok rather than incorrect hash -…

921db50

… once hdk issue is sorted - this should assert for match of post

lucksus reviewed May 15, 2019

View reviewed changes

Added query_type parameter to get_posts_by_topic & fixed test.js file…

fc87d6c

… t.ok assertion.

willemolding added 3 commits May 20, 2019 17:24

adds test for validation based on tag. Updates docstrings in hdk

298c9d9

adds network name to tests so they dont interfere

467f95b

fmt

7073511

lucksus suggested changes May 20, 2019

View reviewed changes

CHANGELOG-UNRELEASED.md Show resolved Hide resolved

willemolding added 3 commits May 21, 2019 11:02

merge with develop

487a15d

adds tests cases for retrieving exact match both app spec and rust test

3afc821

adds optional type also

69039c1

willemolding requested a review from lucksus May 21, 2019 02:25

lucksus reviewed May 21, 2019

View reviewed changes

Re-add mistakenly removed changelog items

40f6df8

lucksus approved these changes May 21, 2019

View reviewed changes

lucksus and others added 2 commits May 21, 2019 14:44

Merge branch 'develop' into add-dynamic-link-tag-core

65ae9d7

Merge branch 'develop' into add-dynamic-link-tag-core

39a828d

zippy approved these changes May 21, 2019

View reviewed changes

willemolding and others added 12 commits May 22, 2019 07:08

adds breaking change note to changelog

7d47d7d

merge latest develop

2502831

fix type error in core_types/links

b4baf11

update comment describing GetLinksKey

82d8f95

update first_steps now links work slightly differently

96745b3

Update hdk-rust/src/api/get_links.rs

59da60e

Co-Authored-By: Eric Harris-Braun <[email protected]>

update docs in api_functions

76fa33f

merge changes made on GH

c62fcc8

fmt

76badd7

fixed typo

bd232f1

fix weird testing thing in dht_reducers

882b25c

Merge branch 'add-dynamic-link-tag-core' of https://github.com/holoch…

976a65d

…ain/holochain-rust into add-dynamic-link-tag-core

willemolding merged commit 76a8d2b into develop May 22, 2019

pdaoust reviewed May 24, 2019

View reviewed changes

zippy deleted the add-dynamic-link-tag-core branch October 4, 2019 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dynamic link tagging #1402

Add dynamic link tagging #1402

willemolding commented May 14, 2019 •

edited

Loading

willemolding May 14, 2019

lucksus May 15, 2019

willemolding May 20, 2019

lucksus May 20, 2019

lucksus left a comment

willemolding commented May 21, 2019

lucksus May 21, 2019

willemolding May 21, 2019

lucksus left a comment

zippy left a comment

pdaoust May 23, 2019

Add dynamic link tagging #1402

Add dynamic link tagging #1402

Conversation

willemolding commented May 14, 2019 • edited Loading

PR summary

willemolding May 14, 2019

Choose a reason for hiding this comment

lucksus May 15, 2019

Choose a reason for hiding this comment

willemolding May 20, 2019

Choose a reason for hiding this comment

lucksus May 20, 2019

Choose a reason for hiding this comment

lucksus left a comment

Choose a reason for hiding this comment

willemolding commented May 21, 2019

lucksus May 21, 2019

Choose a reason for hiding this comment

willemolding May 21, 2019

Choose a reason for hiding this comment

lucksus left a comment

Choose a reason for hiding this comment

zippy left a comment

Choose a reason for hiding this comment

pdaoust May 23, 2019

Choose a reason for hiding this comment

willemolding commented May 14, 2019 •

edited

Loading