urbit/shrub
1
1
Fork 0
mirror of https://github.com/urbit/shrub.git synced 2024-11-28 22:33:06 +03:00
Code Issues Packages Projects Releases Wiki Activity

groups: fix race condition in joining flow

Browse Source 
Attempting to sync a group that a ship is not in causes the subscription
to fail. Because the %add-members action and the sync used to be sent in
one invocation, ames does not guaranteed the order of these remote
actions and so we wait for the %add-members poke to be acked before
adding the group and associated resources.
This commit is contained in:
Liam Fitzgerald 2020-05-29 11:53:14 +10:00
parent 5016e84c0a
commit 2bafd3eb57
8 changed files with 226 additions and 178 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
pkg/arvo/app/chat-view.hoon
View File

@ -114,7 +114,21 @@
~/ %chat-view-agent
|= [=wire =sign:agent:gall]
^- (quip card _this)
?+ -.sign (on-agent:def wire sign)
?+ -.sign (on-agent:def wire sign)
%poke-ack
?. ?=([%join-group @ @ @ @ ~] wire)
(on-agent:def wire sign)
?~ p.sign
(on-agent:def wire sign)
=/ =ship
(slav %p i.t.wire)
=/ ask-history=?
=('y' i.t.t.wire)
=/ =group-id
(need (group-id:de-path:group-store t.t.t.wire))
:_ this
(joined-group:cc group-id ship ask-history)
::
%kick
:_ this
[%pass / %agent [our.bol %chat-store] %watch /updates]~
@ -236,14 +250,21 @@
%join
:: joining unmanaged chat if we don't have the group already
=/ group-path
(fall (maybe-group-from-chat app-path.act) app-path.act)
(maybe-group-from-chat app-path.act)
?^ group-path
~[(chat-hook-poke %add-synced ship.act app-path.act ask-history.act)]
=/ =group-id
(need (group-id:de-path:group-store group-path))
:~ (group-proxy-poke %add-members group-id (sy our.bol ~) ~)
(group-hook-poke %add group-id)
(chat-hook-poke [%add-synced ship.act app-path.act ask-history.act])
(metadata-hook-poke [%add-synced ship.act group-path])
==
(need (group-id:de-path:group-store app-path.act))
=/ =cage
:- %group-action
!> ^- action:group-store
[%add-members group-id (sy our.bol ~) ~]
:: we need this info in the wire to continue the flow after the
:: poke ack
=/ =wire
:- %join-group
[(scot %p ship.act) ?:(ask-history.act %y %n) app-path.act]
[%pass wire %agent [ship.group-id %group-hook] %poke cage]~
::
%groupify
=* app-path app-path.act
@ -298,7 +319,7 @@
::
:-
?: =(path app-path)
(group-poke %add-group group-id policy %.n)
(group-poke %add-group group-id policy %.y)
(contact-view-poke %create term.group-id policy title desc)
%+ murn ~(tap in ships)
|= =ship
@ -338,15 +359,6 @@
^- card
[%pass / %agent [our.bol %metadata-store] %poke %metadata-action !>(act)]
::
++ metadata-hook-poke
|= act=metadata-hook-action
^- card
:* %pass / %agent
[our.bol %metadata-hook]
%poke %metadata-hook-action
!>(act)
==
::
++ send-invite
|= [group-path=path app-path=path =ship]
^- card
@ -418,6 +430,17 @@
?~ meta !!
=(our.bol creator.u.meta)
--
:: +joined-group: Successfully joined unmanaged group, continue flow
::
++ joined-group
|= [=group-id =ship ask-history=?]
^- (list card)
=/ =path
(group-id:en-path:group-store group-id)
:~ (group-hook-poke %add group-id)
(chat-hook-poke %add-synced ship path ask-history)
(metadata-hook-poke %add-synced ship path)
==
::
++ diff-chat-update
|= upd=update:store
@ -469,6 +492,15 @@
%poke %permission-group-hook-action !>(act)
==
::
++ metadata-hook-poke
|= act=metadata-hook-action
^- card
:* %pass / %agent
[our.bol %metadata-hook]
%poke %metadata-hook-action
!>(act)
==
::
++ envelope-scry
|= pax=path
^- (list envelope:store)

30
pkg/arvo/app/contact-hook.hoon
View File

@ -2,6 +2,7 @@
::
/- group-hook,
*contact-hook,
*contact-view,
*invite-store,
*metadata-hook,
*metadata-store,
@ -338,23 +339,10 @@
^- (quip card _state)
|^
?+ -.fact [~ state]
%add-members (add-members +.fact)
%initial-group (initial-group +.fact)
%remove-members (remove +.fact)
%remove-group (unbundle +.fact)
==
++ add-members
|= [=group-id ships=(set ship) tags=(set tag)]
^- (quip card _state)
=/ =path
(group-id:en-path:group-store group-id)
=/ owner (~(get by synced) path)
?~ owner [~ state]
?. =(u.owner our.bol) [~ state]
:_ state
%+ turn ~(tap in (~(del in ships) our.bol))
|= =ship
(send-invite-poke path ship)
::
++ initial-group
|= [=group-id =group]
@ -416,17 +404,10 @@
^- (quip card _state)
?+ -.fact [~ state]
%accepted
=/ changes
(poke-hook-action [%add-synced ship.invite.fact path.invite.fact])
=/ =group-id
(need (group-id:de-path:group-store path.invite.fact))
:-
%+ welp
:~ (group-hook-poke %add group-id)
(metadata-hook-poke [%add-synced ship.invite.fact path.invite.fact])
==
-.changes
+.changes
:_ state
~[(contact-view-poke %join group-id)]
==
::
++ group-hook-poke
@ -444,6 +425,11 @@
^- card
[%pass / %agent [our.bol %contact-store] %poke %contact-action !>(act)]
::
++ contact-view-poke
|= act=contact-view-action
^- card
[%pass / %agent [our.bol %contact-view] %poke %contact-view-action !>(act)]
::
++ group-poke
|= act=action:group-store
^- card

43
pkg/arvo/app/contact-view.hoon
View File

@ -93,6 +93,14 @@
|= [=wire =sign:agent:gall]
^- (quip card _this)
?+ -.sign (on-agent:def wire sign)
%poke-ack
?. ?=([%join-group @ @ ~] wire)
(on-agent:def wire sign)
?^ p.sign
(on-agent:def wire sign)
:_ this
(joined-group:cc t.wire)
::
%kick
[[%pass / %agent [our.bol %contact-store] %watch /updates]~ this]
::
@ -135,27 +143,36 @@
[our.bol name.act]
=/ =path
(group-id:en-path:group-store group-id)
%+ weld
;: weld
:~ (group-poke [%add-group group-id policy.act %.y])
(contact-poke [%create path])
(contact-hook-poke [%add-owned path])
==
(create-metadata path title.act description.act)
(create-metadata path title.act description.act)
?. ?=(%invite -.policy.act)
~
%+ turn
~(tap in pending.policy.act)
|= =ship
(send-invite our.bol %contacts path ship '')
==
::
%join
=/ =path
(group-id:en-path:group-store group-id.act)
:~ (group-listen-hook-poke [%add group-id.act])
(group-proxy-poke %add-members group-id.act (sy our.bol ~) ~)
(contact-hook-poke [%add-synced ship.group-id.act path])
(sync-metadata ship.group-id.act path)
==
=/ =cage
:- %group-action
!> ^- action:group-store
[%add-members group-id.act (sy our.bol ~) ~]
=/ =wire
[%join-group path]
[%pass wire %agent [ship.group-id.act %group-hook] %poke cage]~
::
%invite
=* group-id group-id.act
=/ =path
(group-id:en-path:group-store group-id)
:~ (send-invite ship.group-id %groups path ship.act text.act)
:~ (send-invite ship.group-id %contacts path ship.act text.act)
(add-pending group-id ship.act)
==
::
@ -216,6 +233,16 @@
==
==
::
++ joined-group
|= =path
^- (list card)
=/ =group-id
(need (group-id:de-path:group-store path))
:~ (group-listen-hook-poke [%add group-id])
(contact-hook-poke [%add-synced ship.group-id path])
(sync-metadata ship.group-id path)
==
::
:: +utilities
::
++ add-pending

88
pkg/arvo/app/group-hook.hoon
View File

@ -71,7 +71,6 @@
^- (quip card _this)
=^ cards state
?+ -.wire (on-agent:def:gc wire sign)
%invites (take-invite-sign:gc wire sign)
%store (take-store-sign:gc wire sign)
%proxy (take-proxy-sign:gc wire sign)
==
@ -141,9 +140,7 @@
=. listening
(~(put in listening) group-id)
:_ state
:~ (listen-group group-id)
(add-self group-id)
==
~[(listen-group group-id)]
++ remove
|= =group-id
^- (quip card _state)
@ -157,48 +154,7 @@
:: |signs: signs from agents
::
:: +| %signs
++ take-invite-sign
|= [=wire =sign:agent:gall]
^- (quip card _state)
|^
?+ -.sign (on-agent:def wire sign)
%kick [~[watch-invites] state]
::
%fact
?. =(%invite-update p.cage.sign)
[~ state]
(fact !<(invite-update q.cage.sign))
==
++ fact
|= update=invite-update
^- (quip card _state)
?+ -.update [~ state]
%invite
=* invite invite.update
?. =(our.bol ship.invite)
[~ state]
=/ =group-id
(need (group-id:de-path:store path.invite))
=/ =cage
:- %group-update
!> ^- update:store
[%change-policy group-id [%add-invites (sy recipient.invite ~)]]
:_ state
[%pass [%store path.invite] %agent [our.bol %group-store] %poke cage]~
%accepted
=* invite invite.update
?. =(our.bol ship.invite)
[~ state]
=/ =group-id
(need (group-id:de-path:store path.update))
=/ =cage
:- %group-update
!> ^- update:store
[%add-members group-id (sy recipient.invite ~) ~]
:_ state
[%pass [%store path.invite] %agent [our.bol %group-store] %poke cage]~
==
--
::
:: +take-proxy-sign: take sign from foreign %group-hook
::
++ take-proxy-sign
@ -279,56 +235,39 @@
--
:: +listen-group: Start a new subscription to the proxied .group-id
::
:: Wire is required to prevent a race condition, see +join-group in
:: lib/group
++ listen-group
|= =group-id
^- card
=/ pax=path
=/ pax=path
(group-id:en-path:store group-id)
[%pass [%proxy pax] %agent [ship.group-id %group-hook] %watch [%groups pax]]
:: +add-self: Add self to group
++ add-self
|= =group-id
^- card
=/ pax=path
(group-id:en-path:store group-id)
=/ =cage
:- %group-action
!> ^- action:store
[%add-members group-id (sy our.bol ~) ~]
[%pass [%proxy pax] %agent [ship.group-id %group-hook] %poke cage]
:: +leave-group: Leave a foreign group
=/ =wire
[%proxy pax]
[%pass wire %agent [ship.group-id %group-hook] %watch [%groups pax]]
:: +leave-group: Stop subscribing to a foreign group
::
++ leave-group
|= =group-id
^- (list card)
=/ pax=path
(group-id:en-path:store group-id)
:~ [%pass [%proxy pax] %agent [ship.group-id %group-hook] %leave ~]
[%pass [%store pax] %agent [our.bol %group-store] %poke %group-update !>([%remove-group group-id ~])]
[%pass [%proxy pax] %agent [ship.group-id %group-hook] %poke %group-action !>([%remove-members group-id (sy our.bol ~)])]
==
:: +store-leave-group: Remove a foreign group from our group-store
::
++ store-leave-group
|= =group-id
^- card
=/ pax=path
(group-id:en-path:store group-id)
[%pass [%store pax] %agent [our.bol %group-store] %poke %group-update !>([%remove-group group-id ~])]
=/ =wire
[%proxy pax]
[%pass wire %agent [ship.group-id %group-hook] %leave ~]~
:: +should-proxy-poke: Check if poke should be proxied
::
:: We only allow users to add and remove themselves.
++ should-proxy-poke
|= =update:store
^- ?
=- ~& - -
?: ?=(%initial -.update)
%.n
|^
=/ role=(unit (unit role-tag))
(role-for-ship:grp group-id.update src.bol)
?~ role
%.n
non-member
?~ u.role
member
?- u.u.role
@ -435,6 +374,7 @@
!> ^- update:store
[%initial-group group-id group]
[%give %fact ~ cage]~
::
++ scry-initial
|= =group-id
^- (unit group)

10
pkg/arvo/app/group-store.hoon
View File

@ -397,20 +397,22 @@
--
++ merge-tags
|= [=tags ships=(set ship) tags=(set tag)]
|= [=tags ships=(set ship) new-tags=(set tag)]
^+ tags
=/ tags-list ~(tap in tags)
=/ tags-list ~(tap in new-tags)
|-
?~ tags-list
tags
=* tag i.tags-list
=/ old-ships=(set ship)
(~(gut by tags) tag ~)
%= $
tags-list t.tags-list
::
tags
%+ ~(put by tags)
p.tag
(~(uni in q.tag) ships)
tag
(~(uni in old-ships) ships)
==
++ remove-tags
|= [=group ships=(set ship)]

145
pkg/arvo/app/publish.hoon
View File

@ -406,6 +406,12 @@
^- (quip card _this)
?- -.sin
%poke-ack
?: ?=([%join-group @ @ ~] wir)
=/ =ship
(slav %p i.t.wir)
=^ cards state
(subscribe-notebook ship i.t.t.wir)
[cards this]
?~ p.sin
[~ this]
=^ cards state
@ -924,18 +930,33 @@
%accepted
?> ?=([%notebook @ ~] path.invite.upd)
=/ book i.t.path.invite.upd
=/ wir=wire /subscribe/(scot %p ship.invite.upd)/[book]
=? tile-num (gth tile-num 0)
(dec tile-num)
=/ jon=json (frond:enjs:format %notifications (numb:enjs:format tile-num))
=/ =group-id
[ship.invite.upd book]
(need (group-id:de-path:group-store path.invite.upd))
=/ group
(group-from-book path.invite.upd)
?^ group
(subscribe-notebook ship.invite.upd book)
=/ join-wire=wire
/join-group/[(scot %p ship.invite.upd)]/[book]
=/ =cage
:- %group-action
!> ^- action:group-store
[%add-members group-id (sy our.bol ~) ~]
:_ state
:~ (group-proxy-poke ship.invite.upd %add-members group-id (sy our.bol ~) ~)
(group-hook-poke %add group-id)
[%pass wir %agent [ship.invite.upd %publish] %watch path.invite.upd]
[%give %fact [/publishtile]~ %json !>(jon)]
==
[%pass join-wire %agent [ship.group-id %group-hook] %poke cage]~
==
::
++ subscribe-notebook
|= [=ship book=@tas]
^- (quip card _state)
=/ pax=path /notebook/[book]
=/ wir=wire /subscribe/[(scot %p ship)]/[book]
=? tile-num (gth tile-num 0)
(dec tile-num)
=/ jon=json (frond:enjs:format %notifications (numb:enjs:format tile-num))
:_ state
:~ [%pass wir %agent [ship %publish] %watch pax]
[%give %fact [/publishtile]~ %json !>(jon)]
==
::
++ watch-notebook
@ -952,19 +973,26 @@
::
++ our-beak /(scot %p our.bol)/[q.byk.bol]/(scot %da now.bol)
::
++ book-writers
|= [host=@p book=@tas]
^- (set ship)
=/ =notebook (~(got by books) host book)
=/ =group-id
(need (group-id:de-path:group-store writers.notebook))
%- ~(uni in (fall (scry-tag:grup group-id %admin) ~))
%+ fall
(scry-tag:grup group-id `tag`[%publish (cat 3 %writers- book)])
~
::
++ allowed
|= [who=@p mod=?(%read %write) book=@tas]
^- ?
=/ book=notebook (~(got by books) our.bol book)
=/ =notebook (~(got by books) our.bol book)
=/ =group-id
(need (group-id:de-path:group-store writers.book))
=/ role=(unit (unit role-tag))
(role-for-ship:grup group-id who)
?~ role
%.n
?. ?=(%write mod)
%.y
?=(^ u.role)
(need (group-id:de-path:group-store writers.notebook))
?: ?=(%read mod)
(~(has in (members:grup group-id)) who)
(~(has in (book-writers our.bol book)) who)
::
++ write-file
|= [pax=path cay=cage]
@ -1660,9 +1688,16 @@
::
%subscribe
?> (team:title our.bol src.bol)
=/ wir=wire /subscribe/(scot %p who.act)/[book.act]
=/ join-wire=wire
/join-group/[(scot %p who.act)]/[book.act]
=/ =group-id
[who.act book.act]
=/ =cage
:- %group-action
!> ^- action:group-store
[%add-members group-id (sy our.bol ~) ~]
:_ state
[%pass wir %agent [who.act %publish] %watch /notebook/[book.act]]~
[%pass join-wire %agent [who.act %group-hook] %poke cage]~
:: %unsubscribe
::
%unsubscribe
@ -1833,32 +1868,31 @@
|= [group-path=path app-path=path =metadata]
^- (list card)
[(metadata-poke [%add group-path [%publish app-path] metadata])]~
::
++ group-from-book
|= app-path=path
^- (unit path)
?. .^(? %gu (scot %p our.bol) %metadata-store (scot %da now.bol) ~)
?: ?=([@ ^] app-path)
~& [%assuming-ported-legacy-publish app-path]
`[%'~' app-path]
~&([%weird-publish app-path] ~)
=/ resource-indices
.^ (jug resource group-path)
%gy
(scot %p our.bol)
%metadata-store
(scot %da now.bol)
/resource-indices
==
=/ groups=(unit (set path))
(~(get by resource-indices) [%publish app-path])
?~ groups ~
=/ group-paths ~(tap in u.groups)
?~ group-paths ~
`i.group-paths
--
::
++ group-from-book
|= app-path=path
^- (unit path)
?. .^(? %gu (scot %p our.bol) %metadata-store (scot %da now.bol) ~)
?: ?=([@ ^] app-path)
~& [%assuming-ported-legacy-publish app-path]
`[%'~' app-path]
~&([%weird-publish app-path] ~)
=/ resource-indices
.^ (jug resource group-path)
%gy
(scot %p our.bol)
%metadata-store
(scot %da now.bol)
/resource-indices
==
=/ groups=(unit (set path))
(~(get by resource-indices) [%publish app-path])
?~ groups ~
=/ group-paths ~(tap in u.groups)
?~ group-paths ~
`i.group-paths
::
++ metadata-hook-poke
|= act=metadata-hook-action
^- card
@ -2053,6 +2087,19 @@
?. =(book i.t.pax) out
[[%s (scot %p who)] out]
::
++ get-writers-json
|= [host=@p book=@tas]
=/ =tag
[%publish (cat 3 %writers- book)]
^- json
=/ writers=(list ship)
~(tap in (book-writers host book))
:- %a
%+ turn writers
|= who=@p
^- json
[%s (scot %p who)]
::
++ get-notebook-json
|= [host=@p book-name=@tas]
^- (unit json)
@ -2067,6 +2114,8 @@
=. p.notebook-json
(~(put by p.notebook-json) %subscribers (get-subscribers-json book-name))
=/ notebooks-json (notebooks-map:enjs our.bol books)
=. p.notebook-json
(~(put by p.notebook-json) %writers (get-writers-json host book-name))
?> ?=(%o -.notebooks-json)
=/ host-books-json (~(got by p.notebooks-json) (scot %p host))
?> ?=(%o -.host-books-json)
@ -2178,7 +2227,9 @@
(~(uni by p.notebook-json) (notes-page:enjs notes.u.book 0 50))
=. p.notebook-json
(~(put by p.notebook-json) %subscribers (get-subscribers-json book-name))
=/ jon=json
=. p.notebook-json
(~(put by p.notebook-json) %writers (get-writers-json u.host book-name))
=/ jon=json (pairs notebook+notebook-json ~)
(frond:enjs:format %publish-response (pairs notebook+notebook-json ~))
(json-response:gen (json-to-octs jon))
::
@ -2194,7 +2245,7 @@
=/ note=(unit note) (~(get by notes.u.book) note-name)
?~ note not-found:gen
=/ jon=json
%+ frond %publish-response:enjs:format
%+ frond %publish-response
o+(note-presentation:enjs u.book note-name u.note)
(json-response:gen (json-to-octs jon))
==

8
pkg/arvo/lib/group-store.hoon
View File

@ -134,8 +134,8 @@
?@ tag
(frond %tag s+tag)
%- pairs
:~ tag+s+tag.tag
app+s+app.tag
:~ app+s+app.tag
tag+s+tag.tag
==
::
++ policy
@ -281,8 +281,8 @@
!!
%. json
%- ot
:~ tag+so
app+so
:~ app+so
tag+so
==
:: move to zuse also

12
pkg/arvo/lib/group.hoon
View File

@ -1,6 +1,7 @@
/- *group, *metadata-store
/- *group, *metadata-store, hook=group-hook
/+ store=group-store
|_ =bowl:gall
+$ card card:agent:gall
++ scry-for
|* [=mold =path]
.^ mold
@ -10,6 +11,14 @@
(scot %da now.bowl)
(snoc `^path`path %noun)
==
++ scry-tag
|= [=group-id =tag]
^- (unit (set ship))
=/ group
(scry-group group-id)
?~ group
~
`(~(gut by tags.u.group) tag ~)
::
++ scry-group-path
|= =path
@ -80,6 +89,7 @@
%+ can-join-from-path
(group-id:en-path:store group-id)
ship
::
++ is-managed-path
|= =path
^- ?