From 7e0af9cfca5b898f9fb642f33e8e7d741e24293f Mon Sep 17 00:00:00 2001 From: Liam Fitzgerald Date: Mon, 11 Jan 2021 09:51:36 +1000 Subject: [PATCH 1/5] graph-store: add node existence query --- pkg/arvo/app/graph-store.hoon | 9 +++++++++ pkg/arvo/lib/graph.hoon | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/pkg/arvo/app/graph-store.hoon b/pkg/arvo/app/graph-store.hoon index 46eb4cae18..de9c359aa6 100644 --- a/pkg/arvo/app/graph-store.hoon +++ b/pkg/arvo/app/graph-store.hoon @@ -868,6 +868,15 @@ |= [=atom =node:store] ^- [index:store node:store] [~[atom] node] + :: + [%x %node-exists @ @ @ *] + =/ =ship (slav %p i.t.t.path) + =/ =term i.t.t.t.path + =/ =index:store + (turn t.t.t.t.path (cury slav %ud)) + =/ node=(unit node:store) + (get-node ship term index) + ``noun+!>(?=(^ node)) :: [%x %node @ @ @ *] =/ =ship (slav %p i.t.t.path) diff --git a/pkg/arvo/lib/graph.hoon b/pkg/arvo/lib/graph.hoon index fc84be6fc9..6d78a206f9 100644 --- a/pkg/arvo/lib/graph.hoon +++ b/pkg/arvo/lib/graph.hoon @@ -49,6 +49,14 @@ ?> ?=(^ nodes.q.update) q.n.nodes.q.update :: +++ check-node-existence + |= [res=resource =index:store] + ^- ? + %+ scry-for ,? + %+ weld + /node-exists/(scot %p entity.res)/[name.res] + (turn index (cury scot %ud)) +:: ++ get-update-log |= rid=resource ^- update-log:store From 18301fa7d71d7e76fa633b3f48d11a9f954f8c1c Mon Sep 17 00:00:00 2001 From: Liam Fitzgerald Date: Mon, 11 Jan 2021 09:51:53 +1000 Subject: [PATCH 2/5] hark-store: cleanup dangling unreads Creates a noun poke %fix-dangling, which iterates through the unreads and dismiss those which refer to nodes which no longer exist. Additionally, adds a migration which runs this poke. --- pkg/arvo/app/hark-store.hoon | 60 +++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 7 deletions(-) diff --git a/pkg/arvo/app/hark-store.hoon b/pkg/arvo/app/hark-store.hoon index 031a20284e..e048c4d4fb 100644 --- a/pkg/arvo/app/hark-store.hoon +++ b/pkg/arvo/app/hark-store.hoon @@ -21,13 +21,13 @@ $% state:state-zero:store state:state-one:store state-2 + state-3 == +$ unread-stats [indices=(set index:graph-store) last=@da] :: -+$ state-2 - $: %2 - unreads-each=(jug stats-index:store index:graph-store) ++$ base-state + $: unreads-each=(jug stats-index:store index:graph-store) unreads-count=(map stats-index:store @ud) last-seen=(map stats-index:store @da) =notifications:store @@ -36,8 +36,14 @@ dnd=_| == :: ++$ state-2 + [%2 base-state] +:: ++$ state-3 + [%3 base-state] +:: +$ inflated-state - $: state-2 + $: state-3 cache == :: $cache: useful to have precalculated, but can be derived from state @@ -78,9 +84,16 @@ =| cards=(list card) |^ ?- -.old - %2 + %3 :- cards this(-.state old, +.state (inflate-cache:ha old)) + :: + %2 + %_ $ + -.old %3 + cards :_(cards [%pass / %agent [our dap]:bowl %poke noun+!>(%fix-dangling)]) + == + :: %1 %_ $ @@ -264,10 +277,43 @@ =^ cards state ?+ mark (on-poke:def mark vase) %hark-action (hark-action !<(action:store vase)) - %noun ~& +.state [~ state] + %noun (poke-noun !<(* vase)) == [cards this] :: + ++ poke-noun + |= val=* + ?+ val ~|(%bad-noun-poke !!) + %fix-dangling fix-dangling + %print ~&(+.state [~ state]) + == + :: + ++ fix-dangling + =/ graphs=(set resource) + get-keys:gra + :_ state + %+ roll + ~(tap by unreads-each) + |= $: [=stats-index:store indices=(set index:graph-store)] + out=(list card) + == + ?. ?=(%graph -.stats-index) out + ?. (~(has in graphs) graph.stats-index) + :_(out (poke-us %remove-graph graph.stats-index)) + %+ weld out + ^- (list card) + %+ turn + ^- (list index:graph-store) + %+ skip + `(list index:graph-store)`~(tap in indices) + |=(=index:graph-store (check-node-existence:gra graph.stats-index index)) + |=(=index:graph-store (poke-us %read-each stats-index index)) + :: + ++ poke-us + |= =action:store + ^- card + [%pass / %agent [our dap]:bowl %poke hark-action+!>(action)] + :: ++ hark-action |= =action:store ^- (quip card _state) @@ -636,7 +682,7 @@ == :: ++ inflate-cache - |= state-2 + |= state-3 ^+ +.state =/ nots=(list [p=@da =timebox:store]) (tap:orm notifications) From 1ed047766369741674ee043f59e570274672999d Mon Sep 17 00:00:00 2001 From: Liam Fitzgerald Date: Mon, 11 Jan 2021 11:50:11 +1000 Subject: [PATCH 3/5] graph-store: emit affected descended nodes in %remove-nodes When %remove-nodes is sent as an update, it may refer to a parent node, in which case the deletion of all the parent's children is implied. If we hold onto references to nodes outside of graph-store, we are unable to tell which of our references were affected, other than iterating over every reference to check if one of the deleted indices is a prefix of the reference. This is obviously undesirable, so we emit all of the nodes we deleted, including the nodes deleted implicitly. --- pkg/arvo/app/graph-store.hoon | 64 ++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/pkg/arvo/app/graph-store.hoon b/pkg/arvo/app/graph-store.hoon index de9c359aa6..0fe5b4ea3e 100644 --- a/pkg/arvo/app/graph-store.hoon +++ b/pkg/arvo/app/graph-store.hoon @@ -418,43 +418,83 @@ =/ =update-log:store (~(got by update-logs) resource) =. update-log (put:orm-log update-log time [%0 time [%remove-nodes resource indices]]) + =/ [affected-indices=(set index:store) new-graph=graph:store] + (remove-indices resource graph (sort ~(tap in indices) by-lent)) :: - :- (give [/updates]~ [%remove-nodes resource indices]) + :- (give [/updates]~ [%remove-nodes resource (~(uni in indices) affected-indices)]) %_ state update-logs (~(put by update-logs) resource update-log) graphs %+ ~(put by graphs) resource - [(remove-indices resource graph ~(tap in indices)) mark] + [new-graph mark] == :: + :: we always want to remove the deepest node first, + :: so we don't remove parents before children + ++ by-lent + |* [a=(list) b=(list)] + ^- ? + (gth (lent a) (lent b)) + :: ++ remove-indices + =| affected=(set index:store) |= [=resource:store =graph:store indices=(list index:store)] - ^- graph:store - ?~ indices graph + ^- [(set index:store) graph:store] + ?~ indices [affected graph] + =^ new-affected graph + (remove-index graph i.indices) %_ $ indices t.indices - graph (remove-index graph i.indices) + affected (~(uni in affected) new-affected) + == + :: + ++ get-descendants + |= =graph:store + =| indices=(list index:store) + =/ nodes=(list [atom node:store]) + (tap:orm:store graph) + %- ~(gas in *(set index:store)) + |- =* tap-nodes $ + ^+ indices + %- zing + %+ turn + nodes + |= [atom =node:store] + ^- (list index:store) + %+ welp + (limo index.post.node ~) + ?. ?=(%graph -.children.node) + ~ + %_ tap-nodes + nodes (tap:orm p.children.node) == :: ++ remove-index + =| indices=(set index:store) |= [=graph:store =index:store] - ^- graph:store - ?~ index graph + ^- [_indices graph:store] + ?~ index [indices graph] =* atom i.index :: last index in list :: ?~ t.index - +:`[* graph:store]`(del:orm graph atom) + =^ u-val graph (del:orm graph atom) + ?~ u-val `graph + ?. ?=(%graph -.children.u.u-val) + `graph + =/ new-indices + (get-descendants p.children.u.u-val) + [(~(uni in indices) new-indices) graph] =/ =node:store ~| "parent index does not exist to remove a node from!" (need (get:orm graph atom)) ~| "child index does not exist to remove a node from!" ?> ?=(%graph -.children.node) - %^ put:orm - graph - atom - node(p.children $(graph p.children.node, index t.index)) + =^ new-indices p.children.node + $(graph p.children.node, index t.index) + :- (~(uni in indices) new-indices) + (put:orm graph atom node) -- :: ++ add-signatures From dfc3ef0c0e07cc044bc1a1ae98b3d1c8ced9ba75 Mon Sep 17 00:00:00 2001 From: Liam Fitzgerald Date: Mon, 11 Jan 2021 11:55:33 +1000 Subject: [PATCH 4/5] hark-graph-hook: dismiss unreads for a node when it is deleted --- pkg/arvo/app/hark-graph-hook.hoon | 48 ++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/pkg/arvo/app/hark-graph-hook.hoon b/pkg/arvo/app/hark-graph-hook.hoon index 68dfd65e3a..f839ba1612 100644 --- a/pkg/arvo/app/hark-graph-hook.hoon +++ b/pkg/arvo/app/hark-graph-hook.hoon @@ -167,11 +167,42 @@ :: ?(%remove-graph %archive-graph) (remove-graph resource.q.update) + :: + %remove-nodes + (remove-nodes resource.q.update indices.q.update) :: %add-nodes =* rid resource.q.update (check-nodes ~(val by nodes.q.update) rid) == + :: this is awful, but notification kind should always switch + :: on the index, so hopefully doesn't matter + :: TODO: rethink this + ++ remove-nodes + |= [rid=resource indices=(set index:graph-store)] + =/ to-remove + %- ~(gas by *(set [resource index:graph-store])) + (turn ~(tap in indices) (lead rid)) + :_ state(watching (~(dif in watching) to-remove)) + =/ =tube:clay + (get-conversion:ha rid) + %+ roll + ~(tap in indices) + |= [=index:graph-store out=(list card)] + =| =indexed-post:graph-store + =. index.p.indexed-post index + =+ !<(u-notif-kind=(unit notif-kind) (tube !>(indexed-post))) + ?~ u-notif-kind out + =* notif-kind u.u-notif-kind + =/ =stats-index:store + [%graph rid (scag parent-lent.notif-kind index)] + ?. ?=(%each mode.notif-kind) out + :_(out (poke-hark %read-each stats-index index)) + :: + ++ poke-hark + |= =action:store + ^- card + [%pass / %agent [our.bowl %hark-store] %poke hark-action+!>(action)] :: ++ remove-graph |= rid=resource @@ -246,6 +277,15 @@ :: |_ =bowl:gall :: +++ get-conversion + |= rid=resource + ^- tube:clay + =+ %^ scry [our now]:bowl + ,mark=(unit mark) + /gx/graph-store/graph-mark/(scot %p entity.rid)/[name.rid]/noun + ?~ mark + |=(v=vase !>(~)) + (scry-conversion [our now]:bowl q.byk.bowl u.mark) :: ++ give |= [paths=(list path) =update:hook] @@ -288,13 +328,7 @@ update-core(rid r, updates upds, group grp, module mod) :: ++ get-conversion - ^- tube:clay - =+ %^ scry [our now]:bowl - ,mark=(unit mark) - /gx/graph-store/graph-mark/(scot %p entity.rid)/[name.rid]/noun - ?~ mark - |=(v=vase !>(~)) - (scry-conversion [our now]:bowl q.byk.bowl u.mark) + (^get-conversion rid) :: ++ abet ^- (quip card _state) From 9c7ac3773a0438a70a90d785ca97591d81f3c8cb Mon Sep 17 00:00:00 2001 From: Liam Fitzgerald Date: Tue, 12 Jan 2021 08:15:14 +1000 Subject: [PATCH 5/5] hark, graph-store: address L review --- pkg/arvo/app/graph-store.hoon | 18 ++++++++---------- pkg/arvo/app/hark-graph-hook.hoon | 3 ++- pkg/arvo/app/hark-store.hoon | 19 ++++++++++--------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/arvo/app/graph-store.hoon b/pkg/arvo/app/graph-store.hoon index 0fe5b4ea3e..5fa25e3c83 100644 --- a/pkg/arvo/app/graph-store.hoon +++ b/pkg/arvo/app/graph-store.hoon @@ -452,18 +452,16 @@ ++ get-descendants |= =graph:store =| indices=(list index:store) - =/ nodes=(list [atom node:store]) - (tap:orm:store graph) + =/ nodes (tap:orm:store graph) %- ~(gas in *(set index:store)) |- =* tap-nodes $ ^+ indices %- zing - %+ turn - nodes + %+ turn nodes |= [atom =node:store] ^- (list index:store) %+ welp - (limo index.post.node ~) + index.post.node^~ ?. ?=(%graph -.children.node) ~ %_ tap-nodes @@ -473,18 +471,18 @@ ++ remove-index =| indices=(set index:store) |= [=graph:store =index:store] - ^- [_indices graph:store] + ^- [(set index:store) graph:store] ?~ index [indices graph] =* atom i.index :: last index in list :: ?~ t.index - =^ u-val graph (del:orm graph atom) - ?~ u-val `graph - ?. ?=(%graph -.children.u.u-val) + =^ rm-node graph (del:orm graph atom) + ?~ rm-node `graph + ?. ?=(%graph -.children.u.rm-node) `graph =/ new-indices - (get-descendants p.children.u.u-val) + (get-descendants p.children.u.rm-node) [(~(uni in indices) new-indices) graph] =/ =node:store ~| "parent index does not exist to remove a node from!" diff --git a/pkg/arvo/app/hark-graph-hook.hoon b/pkg/arvo/app/hark-graph-hook.hoon index f839ba1612..a791ec7b7a 100644 --- a/pkg/arvo/app/hark-graph-hook.hoon +++ b/pkg/arvo/app/hark-graph-hook.hoon @@ -197,7 +197,8 @@ =/ =stats-index:store [%graph rid (scag parent-lent.notif-kind index)] ?. ?=(%each mode.notif-kind) out - :_(out (poke-hark %read-each stats-index index)) + :_ out + (poke-hark %read-each stats-index index) :: ++ poke-hark |= =action:store diff --git a/pkg/arvo/app/hark-store.hoon b/pkg/arvo/app/hark-store.hoon index e048c4d4fb..c1ae9bb683 100644 --- a/pkg/arvo/app/hark-store.hoon +++ b/pkg/arvo/app/hark-store.hoon @@ -85,13 +85,16 @@ |^ ?- -.old %3 - :- cards + :- (flop cards) this(-.state old, +.state (inflate-cache:ha old)) :: %2 %_ $ -.old %3 - cards :_(cards [%pass / %agent [our dap]:bowl %poke noun+!>(%fix-dangling)]) + :: + cards + :_ cards + [%pass / %agent [our dap]:bowl %poke noun+!>(%fix-dangling)] == :: @@ -289,8 +292,7 @@ == :: ++ fix-dangling - =/ graphs=(set resource) - get-keys:gra + =/ graphs get-keys:gra :_ state %+ roll ~(tap by unreads-each) @@ -300,13 +302,12 @@ ?. ?=(%graph -.stats-index) out ?. (~(has in graphs) graph.stats-index) :_(out (poke-us %remove-graph graph.stats-index)) - %+ weld out - ^- (list card) + %+ welp out %+ turn - ^- (list index:graph-store) %+ skip - `(list index:graph-store)`~(tap in indices) - |=(=index:graph-store (check-node-existence:gra graph.stats-index index)) + ~(tap in indices) + |= =index:graph-store + (check-node-existence:gra graph.stats-index index) |=(=index:graph-store (poke-us %read-each stats-index index)) :: ++ poke-us